feat(mcp): already-explained voucher guard at stage and commit for match_batch_allocate (#2294) (#2346)

* feat(mcp): already-explained voucher guard at stage and commit for match_batch_allocate The dashboard match-batch route refused BATCH_TX_POSSIBLE_DUPLICATE when posted, unlinked vouchers already summed to the bank row (PR #2300), but the MCP door (gnubok_match_batch_allocate staging + commitMatchBatchAllocate) called the RPC with no guard, so an agent could book a Bankgirot aggregate a second time. The detector existed once; the guard lived in one door. One shared decision helper, lib/invoices/already-explained-guard.ts, now sits on top of the existing detectors (no fork) and is called by the dashboard route, the MCP staging tools and the commit executors: - gnubok_match_batch_allocate refuses to stage, coded BATCH_TX_POSSIBLE_DUPLICATE, naming the vouchers, the reconcile_match / link_transaction_to_journal_entry call that resolves the row, and the exact force + expected_journal_entry_ids binding. - commitMatchBatchAllocate runs the same guard before the RPC and re-validates a staged force binding against the set detected at commit, so a stale approval cannot book a duplicate; 409 auto-rejects with the vouchers in result_data. - force + expected_journal_entry_ids on the tool mirror MatchBatchSchema; an honoured override stages with a compliance_warning and, after the booking succeeds, writes BankTransactionDuplicateDismissed to behandlingshistorik (dashboard route included; it only logged before). - gnubok_match_transaction_to_invoice and commitMatchTransactionInvoice get the dashboard's 1:1 soft-duplicate guard (MATCH_INVOICE_POSSIBLE_DUPLICATE / MATCH_INVOICE_FORCE_CANDIDATE_MISMATCH) with force + expected_journal_entry_id; at commit it runs before the storno. - Registry: both duplicate codes gain retryable: false and a remediation. Catalog payload held under the 60K ceiling by trimming the two tools' own descriptions (59 988 measured, ledger entry in payload-size.bench.test.ts). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019SaJfqNi4VmsG8FMKq99G6 * docs(decisions): record the 2026-09-06 ten-issue batch's first-principles choices Carries the DECISIONS.md lines for PRs #2337 #2339 #2340 #2341 #2342 #2343 #2344 #2345 #2346 #2347 in one place so the ten branches do not conflict on this file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019SaJfqNi4VmsG8FMKq99G6 * fix(mcp): refuse an unverifiable forced override, surface a failed duplicate check, validate the binding (#2294 review) Review round on PR #2346 (CodeRabbit + compliance): - guardAlreadyExplained returned 'clear' when the detector threw even with force=true, so a forced 1:N override could book without re-validating expected_journal_entry_ids and left no behandlingshistorik record. It now returns a distinct 'unverifiable' outcome under force (mirrors guardDuplicatePaymentVoucher); the dashboard route, the MCP staging tool and the commit executor all refuse it with the new registry code BATCH_TX_EXPLAINED_CHECK_FAILED (409, retryable, remediation). Regression tests on every caller. - A detector failure without force still fails open at stage time, but no longer silently: the tools track onDetectError and stage a complianceNote, so preview_data.compliance_warning is set on both match_batch_allocate (GenericPreview renders it) and match_transaction_invoice (MatchTransactionInvoicePreview now renders data.compliance_warning through AttnLine). - expected_journal_entry_ids / expected_journal_entry_id are validated at the MCP boundary (array of 1 to 10 non-empty strings / non-empty string) and refused with VALIDATION_ERROR instead of being silently filtered. No schema description text added: catalog payload unchanged. - RoPA: .compliance/ropa.yaml gains bookkeeping.duplicate_dismissal_history for the BankTransactionDuplicateDismissed record (Art. 6(1)(c), BFNAR 2013:2 p. 9.16, retention per BFL 7 kap, stored in processing_history). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:56:36 +02:00
parent 39d409d257
commit cce0de5704
17 changed files with 1970 additions and 40 deletions
@@ -37,6 +37,13 @@ vi.mock('@/lib/invoices/duplicate-payment-detection', () => ({
  detectExplainingVoucherSetForTransaction: mockDetectExplaining,
 }))

+// An honoured force override is written to behandlingshistorik after the RPC
+// succeeds (issue #2294). Mocked so it never touches a service client here.
+const { mockAppendProcessingHistory } = vi.hoisted(() => ({ mockAppendProcessingHistory: vi.fn() }))
+vi.mock('@/lib/processing-history/append', () => ({
+  appendProcessingHistory: mockAppendProcessingHistory,
+}))
+
 vi.mock('@/lib/company/context', () => ({
  requireCompanyId: vi.fn().mockResolvedValue('company-1'),
  getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
@@ -350,6 +357,17 @@ describe('POST /api/transactions/[id]/match-batch: already-explained guard', ()
    const response = await POST(request, createMockRouteParams({ id: TX_UUID }))
    expect(response.status).toBe(200)
    expect(mockSupabase.rpc).toHaveBeenCalledTimes(1)
+    // Never silent: the honoured override leaves a behandlingshistorik
+    // record naming the vouchers it booked over (issue #2294).
+    expect(mockAppendProcessingHistory).toHaveBeenCalledTimes(1)
+    expect(mockAppendProcessingHistory.mock.calls[0][0]).toMatchObject({
+      companyId: 'company-1',
+      aggregateType: 'BankTransaction',
+      aggregateId: TX_UUID,
+      eventType: 'BankTransactionDuplicateDismissed',
+      actor: { type: 'user', id: 'user-1' },
+      payload: { dismissed_journal_entry_ids: [JE_A, JE_B], via: 'dashboard_force' },
+    })
  })

  it('refuses force=true whose ids do not match the set it re-detects', async () => {
@@ -395,4 +413,27 @@ describe('POST /api/transactions/[id]/match-batch: already-explained guard', ()
    const response = await POST(request, createMockRouteParams({ id: TX_UUID }))
    expect(response.status).toBe(200)
  })
+
+  it('refuses force=true when the detector throws: an override that cannot be re-verified is never honoured', async () => {
+    mockDetectExplaining.mockRejectedValue(new Error('ledger scan timed out'))
+    enqueue({ data: [{ id: INV_UUID, document_type: 'invoice' }], error: null })
+
+    const request = createMockRequest(`/api/transactions/${TX_UUID}/match-batch`, {
+      method: 'POST',
+      body: {
+        allocations: [{ kind: 'customer_invoice', invoice_id: INV_UUID, amount: 88250 }],
+        force: true,
+        expected_journal_entry_ids: [JE_A, JE_B],
+      },
+    })
+    const response = await POST(request, createMockRouteParams({ id: TX_UUID }))
+    const { status, body } = await parseJsonResponse<{
+      error: { code: string; details: { reason: string; force_rejected: boolean } }
+    }>(response)
+    expect(status).toBe(409)
+    expect(body.error.code).toBe('BATCH_TX_EXPLAINED_CHECK_FAILED')
+    expect(body.error.details).toEqual({ reason: 'detector_failed', force_rejected: true })
+    expect(mockSupabase.rpc).not.toHaveBeenCalled()
+    expect(mockAppendProcessingHistory).not.toHaveBeenCalled()
+  })
 })
@@ -5,7 +5,11 @@ import { MatchBatchSchema } from '@/lib/api/schemas'
 import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error'
 import { eventBus } from '@/lib/events/bus'
 import { clearSettledBatchAllocationSuggestions } from '@/lib/invoices/clear-settled-batch-allocations'
-import { detectExplainingVoucherSetForTransaction } from '@/lib/invoices/duplicate-payment-detection'
+import {
+  alreadyExplainedDetails,
+  guardAlreadyExplained,
+  recordExplainedOverride,
+} from '@/lib/invoices/already-explained-guard'
 import { ensureInitialized } from '@/lib/init'
 import type { Invoice, SupplierInvoice, Transaction } from '@/types'
 import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message'
@@ -112,37 +116,32 @@ export const POST = withRouteContext(
    // ones, in the case that prompted this). The vouchers that explain the
    // row are on the ledger, so refuse here and hand them back; the dialog
    // links the row to them (1:N, /api/reconciliation/bank/link) instead of
-    // creating a new voucher. Fail-open on a detection error: the guard is
-    // advisory, the RPC remains the atomicity boundary.
-    let explaining: Awaited<ReturnType<typeof detectExplainingVoucherSetForTransaction>> = null
-    try {
-      explaining = await detectExplainingVoucherSetForTransaction(supabase, companyId!, transactionId)
-    } catch (err) {
-      txLog.warn('match-batch: explaining-voucher detection failed', err as Error)
+    // creating a new voucher. The detect + force-binding decision is the
+    // shared helper the MCP staging tool and the pending-operation commit
+    // run too (issue #2294), so the doors cannot drift. Fail-open on a
+    // detection error: the guard is advisory, the RPC remains the atomicity
+    // boundary.
+    const explained = await guardAlreadyExplained(supabase, companyId!, transactionId, validation.data, {
+      onDetectError: (err) => txLog.warn('match-batch: explaining-voucher detection failed', err as Error),
+    })
+    if (explained.status === 'blocked') {
+      return errorResponseFromCode('BATCH_TX_POSSIBLE_DUPLICATE', txLog, {
+        requestId,
+        details: alreadyExplainedDetails(explained),
+      })
    }
-    if (explaining) {
-      const detectedIds = explaining.vouchers.map((v) => v.journal_entry_id).sort()
-      const expectedIds = [...(validation.data.expected_journal_entry_ids ?? [])].sort()
-      const acknowledged =
-        validation.data.force === true &&
-        detectedIds.length === expectedIds.length &&
-        detectedIds.every((id, i) => id === expectedIds[i])
-      if (!acknowledged) {
-        return errorResponseFromCode('BATCH_TX_POSSIBLE_DUPLICATE', txLog, {
-          requestId,
-          details: {
-            vouchers: explaining.vouchers,
-            total: explaining.total,
-            bank_account_number: explaining.bank_account_number,
-            same_date: explaining.same_date,
-            // force=true with a stale or missing set: the caller must re-read.
-            force_rejected: validation.data.force === true,
-          },
-        })
-      }
+    if (explained.status === 'unverifiable') {
+      // force=true but the check could not run: the override cannot be
+      // re-verified, so it is refused rather than waved through.
+      return errorResponseFromCode('BATCH_TX_EXPLAINED_CHECK_FAILED', txLog, {
+        requestId,
+        details: { reason: 'detector_failed', force_rejected: true },
+      })
+    }
+    if (explained.status === 'overridden') {
      txLog.warn('match-batch: already-explained guard bypassed', {
        reason: 'force=true',
-        journalEntryIds: detectedIds,
+        journalEntryIds: explained.set.vouchers.map((v) => v.journal_entry_id),
        userId: user.id,
      })
    }
@@ -235,6 +234,18 @@ export const POST = withRouteContext(
      transactionId,
    )

+    // The override was acted on: leave the durable behandlingshistorik
+    // record (same event the categorize guard writes), never just a log line.
+    if (explained.status === 'overridden') {
+      await recordExplainedOverride(
+        companyId!,
+        transactionId,
+        explained.set,
+        { actor: { type: 'user', id: user.id }, via: 'dashboard_force' },
+        (err) => txLog.warn('match-batch: failed to record override behandlingshistorik', err as Error),
+      )
+    }
+
    return NextResponse.json({
      data: {
        journal_entry_id: result.journal_entry_id,