fix(arcim-migration): dedup underlag per verifikat and sniff file type from bytes (#1065)

First production sweep of /import-documents (921 Bokio receipts) surfaced
two defects that together dropped 7 of 666 resolvable receipts:

- The idempotency key was company-wide (company_id, sha256), but the same
  file content legitimately backs several verifikat (one arrende contract
  attached to each year's arrende voucher, one insurance letter on two
  vouchers). The second and later verifikat silently lost their underlag.
  The key is now (company_id, sha256, journal_entry_id).

- Bokio's uploads list occasionally declares the wrong contentType (a JPEG
  stored as image/png); magic-byte validation then correctly rejects the
  mismatch, failing a perfectly good receipt. The importer now sniffs the
  real format from the bytes (detectFileMagic, now exported from the
  document service) and only falls back to the declared type when no
  signature is recognised. The synthesised filename extension follows the
  effective type.

Signed-off-by: Jonas Hagberg <jonas@lindan.se>

extensions/general/arcim-migration/__tests__/import-documents.test.ts

@@ -8,7 +8,7 @@ import {
   downloadBokioUpload,
   type BokioVoucherRef,
 } from '@/lib/providers/bokio/attachments'
-import { uploadDocument, computeSHA256 } from '@/lib/core/documents/document-service'
+import { uploadDocument, computeSHA256, detectFileMagic } from '@/lib/core/documents/document-service'
 import { importProviderDocuments } from '../lib/import-documents'
 
 // The Bokio client is constructed but never called directly (the attachments
@@ -23,6 +23,7 @@ vi.mock('@/lib/providers/bokio/attachments', () => ({
 vi.mock('@/lib/core/documents/document-service', () => ({
   uploadDocument: vi.fn(),
   computeSHA256: vi.fn(),
+  detectFileMagic: vi.fn(),
   ALLOWED_DOCUMENT_TYPES: ['application/pdf', 'image/jpeg', 'image/png', 'image/webp'],
 }))
 
@@ -32,6 +33,7 @@ const mockFetchVoucherIndex = vi.mocked(fetchBokioVoucherIndex)
 const mockDownload = vi.mocked(downloadBokioUpload)
 const mockUpload = vi.mocked(uploadDocument)
 const mockSha256 = vi.mocked(computeSHA256)
+const mockDetectMagic = vi.mocked(detectFileMagic)
 
 const COMPANY = 'company-1'
 const USER = 'user-1'
@@ -65,6 +67,8 @@ beforeEach(() => {
   } as never)
   // Default: sha256 derived from the bytes so dedup is deterministic.
   mockSha256.mockImplementation(async (buf: ArrayBuffer) => 'sha-' + Buffer.from(buf).toString('utf8'))
+  // Default: no recognisable signature, so the declared contentType is used.
+  mockDetectMagic.mockReturnValue(null)
   mockUpload.mockResolvedValue({ id: 'doc-1' } as never)
 })
 
@@ -76,14 +80,16 @@ const GNUBOK_VOUCHERS = [
 ]
 const UPLOAD = { id: 'up-1', description: 'Kvitto', contentType: 'application/pdf', journalEntryId: 'bokio-je-1' }
 
-function wireBokio(opts: { existingHashes?: { sha256_hash: string }[] } = {}) {
+function wireBokio(
+  opts: { existingAttachments?: { sha256_hash: string; journal_entry_id: string | null }[] } = {},
+) {
   mockFetchUploads.mockResolvedValue([UPLOAD] as never)
   mockFetchVoucherIndex.mockResolvedValue(new Map([['bokio-je-1', VOUCHER_REF]]))
   mockDownload.mockResolvedValue({ bytes: bytesOf('PDFBYTES'), contentType: 'application/octet-stream' })
   return rangeMockSupabase({
     fiscal_periods: PERIODS,
     journal_entries: GNUBOK_VOUCHERS,
-    document_attachments: opts.existingHashes ?? [],
+    document_attachments: opts.existingAttachments ?? [],
   })
 }
 
@@ -102,8 +108,10 @@ describe('importProviderDocuments', () => {
     expect(metadata).toEqual({ upload_source: 'api', journal_entry_id: 'je-1' })
   })
 
-  it('skips a receipt already archived for the company (sha256 idempotency)', async () => {
-    const supabase = wireBokio({ existingHashes: [{ sha256_hash: 'sha-PDFBYTES' }] })
+  it('skips a receipt already archived on the same verifikat (sha256 + journal entry idempotency)', async () => {
+    const supabase = wireBokio({
+      existingAttachments: [{ sha256_hash: 'sha-PDFBYTES', journal_entry_id: 'je-1' }],
+    })
 
     const result = await importProviderDocuments({ supabase, companyId: COMPANY, userId: USER, consentId: 'c1' })
 
@@ -111,6 +119,17 @@ describe('importProviderDocuments', () => {
     expect(mockUpload).not.toHaveBeenCalled()
   })
 
+  it('still archives content already attached to a DIFFERENT verifikat (same contract, several vouchers)', async () => {
+    const supabase = wireBokio({
+      existingAttachments: [{ sha256_hash: 'sha-PDFBYTES', journal_entry_id: 'je-other' }],
+    })
+
+    const result = await importProviderDocuments({ supabase, companyId: COMPANY, userId: USER, consentId: 'c1' })
+
+    expect(result).toMatchObject({ scanned: 1, linked: 1, skipped: 0 })
+    expect(mockUpload).toHaveBeenCalledTimes(1)
+  })
+
   it('counts a receipt as unmatched when no gnubok verifikat resolves', async () => {
     // gnubok has the number in a DIFFERENT fiscal period: must not match.
     const supabase = rangeMockSupabase({
@@ -202,4 +221,45 @@ describe('importProviderDocuments', () => {
     expect(result).toMatchObject({ scanned: 2, linked: 1, failed: 1 })
     expect(mockUpload).toHaveBeenCalledTimes(1)
   })
+
+  it('archives identical content onto each of several verifikat within one run', async () => {
+    const upload2 = { id: 'up-2', description: 'Kontrakt', contentType: 'application/pdf', journalEntryId: 'bokio-je-2' }
+    const supabase = rangeMockSupabase({
+      fiscal_periods: PERIODS,
+      journal_entries: [
+        { id: 'je-1', fiscal_period_id: 'fp-2021', source_voucher_series: 'V', source_voucher_number: 33 },
+        { id: 'je-2', fiscal_period_id: 'fp-2021', source_voucher_series: 'V', source_voucher_number: 34 },
+      ],
+      document_attachments: [],
+    })
+    mockFetchUploads.mockResolvedValue([UPLOAD, upload2] as never)
+    mockFetchVoucherIndex.mockResolvedValue(
+      new Map<string, BokioVoucherRef>([
+        ['bokio-je-1', VOUCHER_REF],
+        ['bokio-je-2', { series: 'V', number: 34, date: '2021-04-01' }],
+      ]),
+    )
+    // Both uploads are the same file content (one arrende contract, two vouchers).
+    mockDownload.mockResolvedValue({ bytes: bytesOf('PDFBYTES'), contentType: 'application/octet-stream' })
+
+    const result = await importProviderDocuments({ supabase, companyId: COMPANY, userId: USER, consentId: 'c1' })
+
+    expect(result).toMatchObject({ scanned: 2, linked: 2, skipped: 0 })
+    expect(mockUpload).toHaveBeenCalledTimes(2)
+    const targets = mockUpload.mock.calls.map((c) => (c[4] as { journal_entry_id: string }).journal_entry_id)
+    expect(targets).toEqual(['je-1', 'je-2'])
+  })
+
+  it('trusts sniffed magic bytes over a wrong declared contentType', async () => {
+    const supabase = wireBokio()
+    // Bokio's uploads list says PNG but the actual bytes are a JPEG.
+    mockFetchUploads.mockResolvedValue([{ ...UPLOAD, contentType: 'image/png' }] as never)
+    mockDetectMagic.mockReturnValue('image/jpeg')
+
+    const result = await importProviderDocuments({ supabase, companyId: COMPANY, userId: USER, consentId: 'c1' })
+
+    expect(result).toMatchObject({ scanned: 1, linked: 1, failed: 0 })
+    const [, , , file] = mockUpload.mock.calls[0]
+    expect(file).toMatchObject({ name: 'Kvitto.jpg', type: 'image/jpeg' })
+  })
 })

extensions/general/arcim-migration/lib/import-documents.ts

@@ -9,10 +9,14 @@
  * service (storage + document_attachments), linked to the journal entry.
  *
  * Guarantees:
- *  - Idempotent: a receipt already archived for this company (same content,
- *    keyed on company_id + sha256) is skipped, so re-runs don't duplicate.
- *    This matters because a receipt linked to a posted verifikat becomes
- *    räkenskapsinformation and is undeletable (BFL 7 kap 2§ / WORM triggers).
+ *  - Idempotent: a receipt already archived for this verifikat (same content
+ *    AND same journal entry, keyed on company_id + sha256 + journal_entry_id)
+ *    is skipped, so re-runs don't duplicate. The pair matters: the same file
+ *    content can legitimately back several verifikat (one arrende contract
+ *    attached to each year's arrende verifikat), so content alone must not
+ *    dedup across vouchers. This matters because a receipt linked to a posted
+ *    verifikat becomes räkenskapsinformation and is undeletable
+ *    (BFL 7 kap 2§ / WORM triggers).
  *  - Best-effort: a per-receipt failure is counted and logged, never thrown,
  *    so one bad download can't abort the sweep.
  *
@@ -34,6 +38,7 @@ import {
 import {
   uploadDocument,
   computeSHA256,
+  detectFileMagic,
   ALLOWED_DOCUMENT_TYPES,
 } from '@/lib/core/documents/document-service'
 import { fetchAllRows } from '@/lib/supabase/fetch-all'
@@ -56,7 +61,7 @@ export interface ImportDocumentsResult {
   scanned: number
   /** Receipts newly archived and linked to their verifikat. */
   linked: number
-  /** Receipts already archived for this company (sha256 match): re-run skip. */
+  /** Receipts already archived for this verifikat (sha256 + journal entry match): re-run skip. */
   skipped: number
   /** Uploads whose Bokio voucher number resolved to no gnubok verifikat. */
   unmatched: number
@@ -142,7 +147,7 @@ export async function importProviderDocuments(
   const client = new BokioClient()
 
   // ── Bulk reads (one round of paged requests each, no per-item N+1) ──
-  const [uploads, voucherIndex, periods, vouchers, existingHashes] = await Promise.all([
+  const [uploads, voucherIndex, periods, vouchers, existingAttachments] = await Promise.all([
     fetchBokioUploads(client, accessToken, providerCompanyId),
     fetchBokioVoucherIndex(client, accessToken, providerCompanyId),
     // A stable `.order('id')` is required: fetchAllRows pages with `.range()`,
@@ -167,10 +172,10 @@ export async function importProviderDocuments(
         .order('id', { ascending: true })
         .range(from, to),
     ),
-    fetchAllRows<{ sha256_hash: string }>(({ from, to }) =>
+    fetchAllRows<{ sha256_hash: string; journal_entry_id: string | null }>(({ from, to }) =>
       supabase
         .from('document_attachments')
-        .select('sha256_hash')
+        .select('sha256_hash, journal_entry_id')
         .eq('company_id', companyId)
         .order('id', { ascending: true })
         .range(from, to),
@@ -187,8 +192,15 @@ export async function importProviderDocuments(
     )
   }
 
-  // Content hashes already archived for this company → idempotent skip set.
-  const seenHashes = new Set(existingHashes.map((r) => r.sha256_hash))
+  // (content, verifikat) pairs already archived → idempotent skip set. Keyed
+  // on hash + journal entry, NOT hash alone: the same content may back
+  // several verifikat and each deserves its own attachment.
+  const attachmentKey = (sha256: string, journalEntryId: string) => `${sha256}|${journalEntryId}`
+  const seenAttachments = new Set(
+    existingAttachments
+      .filter((r) => r.journal_entry_id != null)
+      .map((r) => attachmentKey(r.sha256_hash, r.journal_entry_id as string)),
+  )
 
   // Every upload that carries a journalEntryId is a receipt we're responsible
   // for. Keep them all in scope (don't pre-filter on a resolvable voucher ref)
@@ -240,28 +252,33 @@ export async function importProviderDocuments(
       )
 
       const sha256 = await computeSHA256(bytes)
-      if (seenHashes.has(sha256)) {
+      if (seenAttachments.has(attachmentKey(sha256, journalEntryId))) {
         result.skipped++
         continue
       }
 
-      // Take the declared type from the upload's contentType (the download is
-      // octet-stream). If it isn't an allowed type, store without a declared
-      // type so uploadDocument skips magic validation rather than rejecting.
-      const declaredType =
-        upload.contentType && ALLOWED_DOCUMENT_TYPES.includes(upload.contentType)
+      // Trust the bytes over Bokio's metadata: the uploads list occasionally
+      // declares the wrong contentType (a JPEG stored as image/png), which
+      // would fail magic validation. Sniff the real format first and fall
+      // back to the declared type only when no signature is recognised; if
+      // neither yields an allowed type, store without a declared type so
+      // uploadDocument skips magic validation rather than rejecting.
+      const sniffedType = detectFileMagic(new Uint8Array(bytes))
+      const effectiveType =
+        sniffedType ??
+        (upload.contentType && ALLOWED_DOCUMENT_TYPES.includes(upload.contentType)
           ? upload.contentType
-          : undefined
+          : undefined)
 
       await uploadDocument(
         supabase,
         userId,
         companyId,
-        { name: fileNameFor(upload, ref, upload.contentType), buffer: bytes, type: declaredType },
+        { name: fileNameFor(upload, ref, effectiveType ?? upload.contentType), buffer: bytes, type: effectiveType },
         { upload_source: 'api', journal_entry_id: journalEntryId },
       )
 
-      seenHashes.add(sha256)
+      seenAttachments.add(attachmentKey(sha256, journalEntryId))
       result.linked++
     } catch (err) {
       result.failed++

lib/core/documents/document-service.ts

@@ -63,7 +63,7 @@ export function validateDocumentFile(file: { size: number; type?: string }): str
  * placeholder or summary instead of the real binary file: those uploads
  * succeed at the storage layer but the bytes are unreadable as a PDF/image.
  */
-function detectFileMagic(bytes: Uint8Array): string | null {
+export function detectFileMagic(bytes: Uint8Array): string | null {
   if (bytes.length < 4) return null
   // PDF: %PDF- anywhere in the first 1024 bytes. ISO 32000 readers accept a
   // preamble before the header (Acrobat scans the first 1 KB), and real-world