feat(mcp): bulk-link documents to vouchers in one staged approval (#1411)
gnubok_link_documents_to_vouchers stages up to 300 document-to-verifikat links as a single pending operation, addressed by voucher_series / voucher_number / fiscal_year instead of journal_entry_id UUIDs, for bulk receipt-migration jobs where N separate tools mean N separate approvals. Staging resolves every row server-side and returns a per-row hit or miss, so a systematic offset such as a wrong fiscal_year is visible before anything is approved rather than after N approvals. Only resolved rows enter the staged operation. The WORM precondition and the document lookup are shared with the single-document executor through precheckDocumentLink: a bulk call must enforce exactly the invariants N single calls would, and a second copy of a BFL 5 kap 6 § guard is a copy that keeps the old behaviour when the first is hardened. A batch that links nothing returns 409 instead of a committed no-op. Partial skips stay committed, but an approval-gated operation on räkenskapsinformation must not leave an audit record asserting a run that changed nothing. The tool is search-only: a one-off migration tool does not belong in the default catalog every session pays for in context, and keeping it there pushed the tools/list projection past the 58.5K token ceiling that payload-size.bench.test.ts guards. 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:
bjornbergenheim
co-authored by
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
Claude Fable 5
parent
4300cb711b
commit
3841ab9f54
@@ -241,6 +241,7 @@ export const TOOL_SCOPE_MAP: Record<string, ApiKeyScope> = {
|
||||
gnubok_get_document_content: 'transactions:read',
|
||||
gnubok_attach_document_to_transaction: 'transactions:write',
|
||||
gnubok_link_document_to_voucher: 'bookkeeping:write',
|
||||
gnubok_link_documents_to_vouchers: 'bookkeeping:write',
|
||||
// Körjournal (mileage): trip log reads/writes are payroll surface
|
||||
// (milersättning, 7331); booking the verifikat is a journal write.
|
||||
gnubok_list_mileage_trips: 'payroll:read',
|
||||
|
||||
@@ -1285,6 +1285,148 @@ describe('commitPendingOperation: link_document_to_voucher', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// ─── link_documents_to_vouchers (bulk) ─────────────────────────────
|
||||
|
||||
describe('commitPendingOperation: link_documents_to_vouchers', () => {
|
||||
const baseOp: Partial<PendingOperation> = {
|
||||
operation_type: 'link_documents_to_vouchers',
|
||||
params: {
|
||||
links: [
|
||||
{ document_id: 'doc-1', journal_entry_id: 'je-1', journal_entry_line_id: null },
|
||||
{ document_id: 'doc-2', journal_entry_id: 'je-2', journal_entry_line_id: null },
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
it('happy path: links every row and reports zero skipped', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
// row 1: doc-1 -> je-1
|
||||
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: 'kvitto1.pdf', journal_entry_id: 'je-1', journal_entry_line_id: null },
|
||||
error: null,
|
||||
}) // linkToJournalEntry: doc update
|
||||
// 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
|
||||
enqueue({
|
||||
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 }) // 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({
|
||||
linked_count: 2,
|
||||
skipped_count: 0,
|
||||
})
|
||||
})
|
||||
|
||||
it('mixed batch: a missing document is skipped without blocking the other rows', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
// row 1: doc-1 not found in this company
|
||||
enqueue({ data: null, error: null }) // doc fetch: not found
|
||||
// row 2: doc-2 -> je-2 succeeds
|
||||
enqueue({ data: { id: 'doc-2', journal_entry_id: null }, error: null }) // doc fetch
|
||||
enqueue({ data: { id: 'je-2' }, error: null }) // linkToJournalEntry: JE ownership
|
||||
enqueue({
|
||||
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 }) // dispatcher commit update
|
||||
|
||||
const result = await commitPendingOperation(
|
||||
supabase as never, 'user-1', 'company-1', makePendingOp(baseOp),
|
||||
)
|
||||
// Non-transactional bulk op: overall status is 'committed' even with
|
||||
// partial skips (mirrors bulk_book_inbox_items's booked/skipped split),
|
||||
// since no irreversible side-effect was posted for the skipped row.
|
||||
expect(result.status).toBe('committed')
|
||||
expect(result.data).toMatchObject({ linked_count: 1, skipped_count: 1 })
|
||||
expect((result.data as { skipped: Array<{ document_id: string; reason: string }> }).skipped[0]).toMatchObject({
|
||||
document_id: 'doc-1',
|
||||
reason: 'Bilagan hittades inte.',
|
||||
})
|
||||
})
|
||||
|
||||
it('WORM guard skips a row whose doc is already linked to a different posted JE', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
// row 1: doc-1 already linked to je-OTHER (posted) -> skipped
|
||||
enqueue({ data: { id: 'doc-1', journal_entry_id: 'je-OTHER' }, error: null }) // doc fetch
|
||||
enqueue({ data: { status: 'posted' }, error: null }) // WORM: existing JE status
|
||||
// row 2: doc-2 -> je-2 succeeds
|
||||
enqueue({ data: { id: 'doc-2', journal_entry_id: null }, error: null }) // doc fetch
|
||||
enqueue({ data: { id: 'je-2' }, error: null }) // linkToJournalEntry: JE ownership
|
||||
enqueue({
|
||||
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 }) // 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({ linked_count: 1, skipped_count: 1 })
|
||||
})
|
||||
|
||||
it('auto-rejects 409 when every row is skipped instead of recording a committed no-op', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
// row 1: doc-1 already linked to a posted JE -> WORM skip
|
||||
enqueue({ data: { id: 'doc-1', journal_entry_id: 'je-OTHER' }, error: null }) // doc fetch
|
||||
enqueue({ data: { status: 'posted' }, error: null }) // WORM: existing JE status
|
||||
// row 2: doc-2 not found -> skip
|
||||
enqueue({ data: null, error: null }) // doc fetch: not found
|
||||
enqueue({ data: null, error: null }) // dispatcher reject update
|
||||
|
||||
const result = await commitPendingOperation(
|
||||
supabase as never, 'user-1', 'company-1', makePendingOp(baseOp),
|
||||
)
|
||||
// Partial skips stay 'committed' (see the mixed-batch case), but a batch
|
||||
// that linked nothing must not leave an audit record asserting a run that
|
||||
// changed nothing: the single-document executor returns 409 for the same
|
||||
// conditions, and the bulk path must not be the weaker one.
|
||||
expect(result.status).toBe('rejected')
|
||||
expect(result.http_status).toBe(409)
|
||||
})
|
||||
|
||||
it('fails 400 when the staged params carry an empty links array', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
enqueue({ data: null, error: null }) // dispatcher failure update
|
||||
|
||||
const result = await commitPendingOperation(
|
||||
supabase as never, 'user-1', 'company-1',
|
||||
makePendingOp({ ...baseOp, params: { links: [] } }),
|
||||
)
|
||||
// 400 is not auto-reject territory (only 404 and 409 are), so the operation
|
||||
// is recorded as failed rather than rejected.
|
||||
expect(result.status).toBe('failed')
|
||||
expect(result.http_status).toBe(400)
|
||||
})
|
||||
|
||||
it('fails 400 when links is missing entirely', async () => {
|
||||
const { supabase, enqueue } = createQueuedMockSupabase()
|
||||
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
|
||||
enqueue({ data: null, error: null }) // dispatcher failure update
|
||||
|
||||
const result = await commitPendingOperation(
|
||||
supabase as never, 'user-1', 'company-1',
|
||||
makePendingOp({ ...baseOp, params: {} }),
|
||||
)
|
||||
expect(result.status).toBe('failed')
|
||||
expect(result.http_status).toBe(400)
|
||||
})
|
||||
})
|
||||
|
||||
// ─── categorize_transaction: dimensions propagation (PR7) ──────────
|
||||
|
||||
describe('commitPendingOperation: categorize_transaction: dimensions propagation (PR7)', () => {
|
||||
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
supplierInvoiceSekAmounts,
|
||||
} from '@/lib/currency/supplier-invoice-rate'
|
||||
import { roundOre } from '@/lib/money'
|
||||
import { getErrorMessage } from '@/lib/errors/get-error-message'
|
||||
import { validateVatNumber } from '@/lib/vat/vies-client'
|
||||
import {
|
||||
normalizeVatRateToDecimal,
|
||||
@@ -3181,6 +3182,74 @@ async function commitAttachDocumentToTransaction(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared precondition for linking a document to a verifikat, used by BOTH the
|
||||
* single and the bulk executor. It must live in one place: a bulk call has to
|
||||
* be exactly N single calls, and a second copy of a legally motivated guard is
|
||||
* a copy that silently keeps the old behaviour when the first one is hardened.
|
||||
*
|
||||
* Two checks: the document exists in THIS company, and the WORM rule that a
|
||||
* document already belonging to a POSTED verifikat cannot be moved to another
|
||||
* one (it is räkenskapsinformation under BFL 5 kap 6 § once the verifikat is
|
||||
* posted). Re-linking to the SAME verifikat is allowed: that is a no-op or a
|
||||
* line-level refinement, not a move.
|
||||
*/
|
||||
type DocumentLinkPrecheck =
|
||||
| { ok: true }
|
||||
| { ok: false; reason: string; status: number }
|
||||
|
||||
async function precheckDocumentLink(
|
||||
supabase: SupabaseClient,
|
||||
companyId: string,
|
||||
documentId: string,
|
||||
journalEntryId: string
|
||||
): Promise<DocumentLinkPrecheck> {
|
||||
const { data: doc, error: docError } = await supabase
|
||||
.from('document_attachments')
|
||||
.select('id, file_name, journal_entry_id')
|
||||
.eq('id', documentId)
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
if (docError || !doc) return { ok: false, reason: 'Bilagan hittades inte.', status: 404 }
|
||||
|
||||
const existingJeId = (doc.journal_entry_id as string | null) ?? null
|
||||
if (existingJeId && existingJeId !== journalEntryId) {
|
||||
const { data: existingJe } = await supabase
|
||||
.from('journal_entries')
|
||||
.select('status')
|
||||
.eq('id', existingJeId)
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
if (existingJe && (existingJe as { status: string }).status === 'posted') {
|
||||
return {
|
||||
ok: false,
|
||||
reason:
|
||||
'Bilagan är kopplad till en bokförd verifikation och kan inte länkas om. Ladda upp ett nytt dokument.',
|
||||
status: 409,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return { ok: true }
|
||||
}
|
||||
|
||||
/**
|
||||
* 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,
|
||||
* and the raw trigger text is English database prose. Everything else goes
|
||||
* through getErrorMessage so nothing reaches the Granskning panel untranslated.
|
||||
*/
|
||||
function documentLinkFailureMessage(err: unknown): string {
|
||||
const msg = err instanceof Error ? err.message : String(err ?? '')
|
||||
if (/locked\/closed fiscal period|Bokföringen är låst/i.test(msg)) {
|
||||
return 'Verifikationens period är låst: bilagan kan inte länkas.'
|
||||
}
|
||||
// No ErrorContext value covers document attachments, and journal_entry would
|
||||
// phrase the fallback as a verifikat problem when the failure is the link.
|
||||
// Omitting it keeps the generic Swedish fallback, which is accurate here.
|
||||
return getErrorMessage(err)
|
||||
}
|
||||
|
||||
async function commitLinkDocumentToVoucher(
|
||||
supabase: SupabaseClient,
|
||||
_userId: string,
|
||||
@@ -3194,31 +3263,8 @@ async function commitLinkDocumentToVoucher(
|
||||
return { error: 'document_id and journal_entry_id are required', status: 400 }
|
||||
}
|
||||
|
||||
const { data: doc, error: docError } = await supabase
|
||||
.from('document_attachments')
|
||||
.select('id, file_name, journal_entry_id')
|
||||
.eq('id', documentId)
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
if (docError || !doc) return { error: 'Document not found', status: 404 }
|
||||
|
||||
// WORM guard: refuse to re-link a doc already linked to a DIFFERENT posted JE.
|
||||
const existingJeId = (doc.journal_entry_id as string | null) ?? null
|
||||
if (existingJeId && existingJeId !== journalEntryId) {
|
||||
const { data: existingJe } = await supabase
|
||||
.from('journal_entries')
|
||||
.select('status')
|
||||
.eq('id', existingJeId)
|
||||
.eq('company_id', companyId)
|
||||
.maybeSingle()
|
||||
if (existingJe && (existingJe as { status: string }).status === 'posted') {
|
||||
return {
|
||||
error:
|
||||
'Bilagan är kopplad till en bokförd verifikation och kan inte länkas om. Ladda upp ett nytt dokument.',
|
||||
status: 409,
|
||||
}
|
||||
}
|
||||
}
|
||||
const precheck = await precheckDocumentLink(supabase, companyId, documentId, journalEntryId)
|
||||
if (!precheck.ok) return { error: precheck.reason, status: precheck.status }
|
||||
|
||||
try {
|
||||
const updated = await linkToJournalEntry(
|
||||
@@ -3237,14 +3283,90 @@ async function commitLinkDocumentToVoucher(
|
||||
},
|
||||
}
|
||||
} catch (err) {
|
||||
const msg = (err as Error).message ?? ''
|
||||
if (/locked\/closed fiscal period|Bokföringen är låst/i.test(msg)) {
|
||||
return {
|
||||
error: 'Verifikationens period är låst: bilagan kan inte länkas.',
|
||||
status: 409,
|
||||
}
|
||||
const msg = err instanceof Error ? err.message : String(err ?? '')
|
||||
const isLocked = /locked\/closed fiscal period|Bokföringen är låst/i.test(msg)
|
||||
return { error: documentLinkFailureMessage(err), status: isLocked ? 409 : 500 }
|
||||
}
|
||||
}
|
||||
|
||||
async function commitLinkDocumentsToVouchers(
|
||||
supabase: SupabaseClient,
|
||||
_userId: string,
|
||||
companyId: string,
|
||||
params: Record<string, unknown>
|
||||
): Promise<ExecutorResult> {
|
||||
const links = params.links as Array<{
|
||||
document_id: string; journal_entry_id: string; journal_entry_line_id: string | null
|
||||
}> | undefined
|
||||
if (!Array.isArray(links) || links.length === 0) {
|
||||
return { error: 'links is required (non-empty)', status: 400 }
|
||||
}
|
||||
|
||||
const linked: Array<{ document_id: string; journal_entry_id: string }> = []
|
||||
const skipped: Array<{ document_id: string; journal_entry_id: string; reason: string }> = []
|
||||
|
||||
// Sequential, not Promise.all: each row re-validates against the current DB
|
||||
// state (a prior row in the SAME batch can change a doc's journal_entry_id,
|
||||
// and the WORM guard must see that), and goes through the SAME
|
||||
// precheckDocumentLink + linkToJournalEntry pair as the single-document
|
||||
// executor, so a bulk call enforces identical invariants to N individual
|
||||
// calls rather than a parallel reimplementation of them.
|
||||
for (const link of links) {
|
||||
const documentId = link.document_id
|
||||
const journalEntryId = link.journal_entry_id
|
||||
if (!documentId || !journalEntryId) {
|
||||
skipped.push({ document_id: documentId ?? '(saknas)', journal_entry_id: journalEntryId ?? '(saknas)', reason: 'Raden saknar dokument-id eller verifikations-id.' })
|
||||
continue
|
||||
}
|
||||
return { error: `Failed to link document: ${msg}`, status: 500 }
|
||||
|
||||
const precheck = await precheckDocumentLink(supabase, companyId, documentId, journalEntryId)
|
||||
if (!precheck.ok) {
|
||||
skipped.push({ document_id: documentId, journal_entry_id: journalEntryId, reason: precheck.reason })
|
||||
continue
|
||||
}
|
||||
|
||||
try {
|
||||
const updated = await linkToJournalEntry(
|
||||
supabase, companyId, documentId, journalEntryId, link.journal_entry_line_id ?? undefined,
|
||||
)
|
||||
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
|
||||
// sentence: skipped reasons land in result_data and are rendered in the
|
||||
// Granskning panel, so none of them may reach the user as raw English
|
||||
// database prose.
|
||||
skipped.push({ document_id: documentId, journal_entry_id: journalEntryId, reason: documentLinkFailureMessage(err) })
|
||||
}
|
||||
}
|
||||
|
||||
// Nothing linked is a failure, not a partial success. Recording it as
|
||||
// `committed` would leave an approval-gated operation on
|
||||
// räkenskapsinformation sitting in the audit trail asserting a run that
|
||||
// changed nothing, and the single-document executor returns 409 for exactly
|
||||
// these conditions (locked period, WORM guard, missing document). A batch
|
||||
// must not be the weaker path.
|
||||
if (linked.length === 0) {
|
||||
return {
|
||||
error: `Inga bilagor kunde länkas (${skipped.length} hoppades över). Första orsak: ${skipped[0]?.reason ?? 'okänd'}`,
|
||||
status: 409,
|
||||
}
|
||||
}
|
||||
|
||||
log.info('link_documents_to_vouchers committed', {
|
||||
companyId,
|
||||
operationType: 'link_documents_to_vouchers',
|
||||
requested: links.length,
|
||||
linkedCount: linked.length,
|
||||
skippedCount: skipped.length,
|
||||
})
|
||||
|
||||
return {
|
||||
data: {
|
||||
linked_count: linked.length,
|
||||
skipped_count: skipped.length,
|
||||
linked,
|
||||
skipped,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5896,6 +6018,9 @@ async function commitPendingOperationInner(
|
||||
case 'link_document_to_voucher':
|
||||
result = await commitLinkDocumentToVoucher(supabase, userId, companyId, pendingOp.params)
|
||||
break
|
||||
case 'link_documents_to_vouchers':
|
||||
result = await commitLinkDocumentsToVouchers(supabase, userId, companyId, pendingOp.params)
|
||||
break
|
||||
case 'run_year_end':
|
||||
result = await commitRunYearEnd(supabase, userId, companyId, pendingOp.params)
|
||||
break
|
||||
|
||||
@@ -94,6 +94,8 @@ export const OPERATION_RISK_TIERS: Record<string, RiskLevel> = {
|
||||
// (BFL 5 kap 6 §) and becomes immutable once the JE is posted. Medium so a
|
||||
// human confirms the doc-to-verifikat pairing before it locks.
|
||||
link_document_to_voucher: 'medium',
|
||||
// Same rationale as link_document_to_voucher, N rows in one staged op.
|
||||
link_documents_to_vouchers: 'medium',
|
||||
// Dimension-only diff on posted lines (verifikat stays immutable), fully
|
||||
// audited via dimension_retag_log, but it rewrites reporting history, so
|
||||
// it crosses a human at medium.
|
||||
|
||||
Reference in New Issue
Block a user