From f04dc4c4e0bc25d8e5ab162a939e123892e2b437 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:43:02 +0200 Subject: [PATCH] fix(transactions): allow re-linking a bank tx stranded on a reversed verifikat (#988) (#1009) * fix(transactions): allow re-linking a bank tx stranded on a reversed verifikat (#988) A transaction whose journal_entry_id points at a reversed/cancelled entry reads as "utan koppling" in the UI (the transactions page enriches only status='posted' links), yet the re-booking guards treated ANY non-null pointer as "already linked". So a storno'd/corrected transaction could never be linked to another verifikat or re-categorized: the exact symptom in issue #988. Add a shared hasLiveJournalEntryLink() predicate used by every re-booking guard (linkTransactionToJournalEntry, manualLink, categorize-core, and the MCP link stage-check): a pointer at a non-posted entry is treated as re-linkable, and the two optimistic-locked writes now lock on the exact previous pointer (null OR the stale id) instead of always .is(null), so the overwrite goes through race-safely. hasLiveJournalEntryLink fails closed on a read error so a transient blip can't detach a genuinely live link. The source was fixed in #726 (reverseEntry/correctEntry now detach/re-point the tx); this makes the guards self-heal for the pre-#726 backlog and any future best-effort miss. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(transactions): detect 0-row CAS before invoice effects; fix categorize commit test Addresses PR review (CodeRabbit Critical + CI): - link-journal-entry.ts: the tx UPDATE now .select('id') and treats a 0-row result as LINK_TX_TX_ALREADY_LINKED, failing BEFORE any invoice settlement / invoice_payments insert. Without this, a concurrent re-link that lost the CAS would still mark the invoice paid against a transaction we didn't link (same optimistic-lock contract manualLink already enforces). - pending-operations commit route test: the categorize_transaction "already categorized" case now enqueues the hasLiveJournalEntryLink status read (posted = live) so it still returns 409. This was the core-only CI failure: the new liveness read in categorize-core consumed a queued response. - Updated the link happy-path / invoice-race test enqueues to return a row for the now-selecting tx UPDATE. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../[id]/commit/__tests__/route.test.ts | 1 + .../__tests__/route.test.ts | 36 +++++++-- extensions/general/mcp-server/server.ts | 7 +- .../link-transaction-journal-entry.test.ts | 41 +++++++++- .../__tests__/bank-reconciliation.test.ts | 23 +++++- lib/reconciliation/bank-reconciliation.ts | 26 +++++-- .../__tests__/link-journal-entry.test.ts | 54 ++++++++++++++ lib/transactions/categorize-core.ts | 13 +++- lib/transactions/link-journal-entry.ts | 74 ++++++++++++++++++- 9 files changed, 249 insertions(+), 26 deletions(-) create mode 100644 lib/transactions/__tests__/link-journal-entry.test.ts diff --git a/app/api/pending-operations/[id]/commit/__tests__/route.test.ts b/app/api/pending-operations/[id]/commit/__tests__/route.test.ts index 8a1d1dfa..c767a9a4 100644 --- a/app/api/pending-operations/[id]/commit/__tests__/route.test.ts +++ b/app/api/pending-operations/[id]/commit/__tests__/route.test.ts @@ -181,6 +181,7 @@ describe('POST /api/pending-operations/:id/commit', () => { { data: pendingOp }, // fetch pending op { data: { id: 'op-1' } }, // CAS claim { data: tx }, // fetch transaction (already has JE) + { data: { status: 'posted' } }, // hasLiveJournalEntryLink: existing JE is live { data: null, error: null }, // auto-reject update ]) diff --git a/app/api/transactions/[id]/link-journal-entry/__tests__/route.test.ts b/app/api/transactions/[id]/link-journal-entry/__tests__/route.test.ts index 055f3e83..6080280b 100644 --- a/app/api/transactions/[id]/link-journal-entry/__tests__/route.test.ts +++ b/app/api/transactions/[id]/link-journal-entry/__tests__/route.test.ts @@ -71,11 +71,12 @@ describe('POST /api/transactions/[id]/link-journal-entry', () => { expect(body.error.code).toBe('TX_CATEGORIZE_TX_NOT_FOUND') }) - it('returns 400 when transaction is already linked', async () => { + it('returns 400 when transaction is already linked to a LIVE (posted) entry', async () => { enqueue({ data: makeTransaction({ id: TX_UUID, journal_entry_id: 'je-prior' }), error: null, }) + enqueue({ data: { status: 'posted' }, error: null }) // hasLiveJournalEntryLink: prior link is live const request = createMockRequest(`/api/transactions/${TX_UUID}/link-journal-entry`, { method: 'POST', body: { journal_entry_id: JE_UUID }, @@ -86,6 +87,31 @@ describe('POST /api/transactions/[id]/link-journal-entry', () => { expect(body.error.code).toBe('LINK_TX_TX_ALREADY_LINKED') }) + it('re-links a transaction stranded on a reversed entry (#988)', async () => { + // Pointer at a status='reversed' entry reads as "utan koppling" in the UI; + // the link must succeed to another posted verifikat, overwriting the stale id. + enqueue({ + data: makeTransaction({ id: TX_UUID, journal_entry_id: 'je-reversed', amount: 1000, date: '2026-05-15' }), + error: null, + }) + enqueue({ data: { status: 'reversed' }, error: null }) // hasLiveJournalEntryLink: stale link + enqueue({ + data: { id: JE_UUID, status: 'posted', voucher_series: 'A', voucher_number: 7, entry_date: '2026-05-15' }, + error: null, + }) // target JE fetch + enqueue({ data: [{ id: TX_UUID }], error: null }) // tx UPDATE + enqueue({ data: null, error: null }) // logMatchEvent insert + const request = createMockRequest(`/api/transactions/${TX_UUID}/link-journal-entry`, { + method: 'POST', + body: { journal_entry_id: JE_UUID }, + }) + const response = await POST(request, createMockRouteParams({ id: TX_UUID })) + const { status, body } = await parseJsonResponse<{ success: boolean; voucher_label: string }>(response) + expect(status).toBe(200) + expect(body.success).toBe(true) + expect(body.voucher_label).toBe('A-7') + }) + it('returns 404 when journal entry not found', async () => { enqueue({ data: makeTransaction({ id: TX_UUID, journal_entry_id: null }), @@ -145,7 +171,7 @@ describe('POST /api/transactions/[id]/link-journal-entry', () => { error: null, }) // Update transaction - enqueue({ data: null, error: null }) + enqueue({ data: [{ id: TX_UUID }], error: null }) // logMatchEvent insert enqueue({ data: null, error: null }) @@ -199,7 +225,7 @@ describe('POST /api/transactions/[id]/link-journal-entry', () => { error: null, }) // Update transaction - enqueue({ data: null, error: null }) + enqueue({ data: [{ id: TX_UUID }], error: null }) // Update invoice (optimistic lock returns updated row) enqueue({ data: [{ id: INV_UUID }], error: null }) // Insert invoice_payments @@ -303,7 +329,7 @@ describe('POST /api/transactions/[id]/link-journal-entry', () => { error: null, }) // Update transaction succeeds - enqueue({ data: null, error: null }) + enqueue({ data: [{ id: TX_UUID }], error: null }) // Optimistic invoice update returns 0 rows enqueue({ data: [], error: null }) // Compensating rollback: restore prior tx state @@ -345,7 +371,7 @@ describe('POST /api/transactions/[id]/link-journal-entry', () => { error: null, }) // Update transaction succeeds - enqueue({ data: null, error: null }) + enqueue({ data: [{ id: TX_UUID }], error: null }) // Optimistic invoice update succeeds enqueue({ data: [{ id: INV_UUID }], error: null }) // invoice_payments insert fails with non-23505 error diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 30999726..de0a003e 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -12,7 +12,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { buildMappingResultFromCategory } from '@/lib/bookkeeping/category-mapping' import { buildTransactionEntryLines, createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' import { upsertCounterpartyTemplate, findCounterpartyTemplatesBatch, formatCounterpartyName } from '@/lib/bookkeeping/counterparty-templates' -import { formatVoucherLabel } from '@/lib/transactions/link-journal-entry' +import { formatVoucherLabel, hasLiveJournalEntryLink } from '@/lib/transactions/link-journal-entry' import { eventBus } from '@/lib/events/bus' import { getVatRules, getAvailableVatRates } from '@/lib/invoices/vat-rules' import { fetchExchangeRate, convertToSEK } from '@/lib/currency/riksbanken' @@ -6397,7 +6397,10 @@ export const tools: McpTool[] = [ .eq('company_id', companyId) .maybeSingle() if (txError || !tx) throw new Error('Transaction not found') - if (tx.journal_entry_id) { + // Only a live posted link blocks re-linking; a stale pointer at a reversed + // entry (storno/correction) reads as "utan koppling" and must stay + // re-linkable (issue #988). The commit handler re-validates the same way. + if (tx.journal_entry_id && (await hasLiveJournalEntryLink(supabase, companyId, tx.journal_entry_id))) { throw new Error('Transaction is already linked to a journal entry') } diff --git a/lib/pending-operations/__tests__/link-transaction-journal-entry.test.ts b/lib/pending-operations/__tests__/link-transaction-journal-entry.test.ts index 1ea314f6..b71f453c 100644 --- a/lib/pending-operations/__tests__/link-transaction-journal-entry.test.ts +++ b/lib/pending-operations/__tests__/link-transaction-journal-entry.test.ts @@ -84,13 +84,14 @@ describe('commitPendingOperation: link_transaction_journal_entry', () => { expect(result.http_status).toBe(404) }) - it('returns 400 when transaction already linked', async () => { + it('returns 400 when transaction already linked to a LIVE (posted) entry', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim enqueue({ data: makeTransaction({ id: TX_UUID, journal_entry_id: 'je-prior' }), error: null, }) + enqueue({ data: { status: 'posted' }, error: null }) // hasLiveJournalEntryLink: prior link is live enqueue({ data: null, error: null }) // dispatcher's reject update const op = makePendingOp({ @@ -103,6 +104,38 @@ describe('commitPendingOperation: link_transaction_journal_entry', () => { expect(result.error).toMatch(/already linked/i) }) + it('re-links a transaction stranded on a reversed entry (#988)', async () => { + // The tx still points at a status='reversed' entry (a storno/correction left + // the pointer behind). The UI shows it as "utan koppling"; the guard must + // agree and let it be linked to another posted verifikat. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ + data: makeTransaction({ id: TX_UUID, journal_entry_id: 'je-reversed', amount: 1000, date: '2026-05-15' }), + error: null, + }) + enqueue({ data: { status: 'reversed' }, error: null }) // hasLiveJournalEntryLink: stale link + enqueue({ + data: { id: JE_UUID, status: 'posted', voucher_series: 'A', voucher_number: 12, entry_date: '2026-05-15' }, + error: null, + }) // target JE fetch + enqueue({ data: [{ id: TX_UUID }], error: null }) // tx UPDATE (overwrites the stale pointer) + enqueue({ data: null, error: null }) // logMatchEvent insert + enqueue({ data: null, error: null }) // dispatcher commit update + + const op = makePendingOp({ + params: { transaction_id: TX_UUID, journal_entry_id: JE_UUID }, + }) + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('committed') + expect(result.data).toMatchObject({ + transaction_id: TX_UUID, + journal_entry_id: JE_UUID, + voucher_label: 'A-12', + }) + }) + it('returns 400 when JE is not posted', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim @@ -149,7 +182,7 @@ describe('commitPendingOperation: link_transaction_journal_entry', () => { }, error: null, }) - enqueue({ data: null, error: null }) // tx UPDATE + enqueue({ data: [{ id: TX_UUID }], error: null }) // tx UPDATE enqueue({ data: null, error: null }) // logMatchEvent insert enqueue({ data: null, error: null }) // dispatcher commit update @@ -196,7 +229,7 @@ describe('commitPendingOperation: link_transaction_journal_entry', () => { }), error: null, }) - enqueue({ data: null, error: null }) // tx UPDATE + enqueue({ data: [{ id: TX_UUID }], error: null }) // tx UPDATE enqueue({ data: [{ id: INV_UUID }], error: null }) // optimistic-lock invoice UPDATE enqueue({ data: null, error: null }) // invoice_payments INSERT enqueue({ data: null, error: null }) // logMatchEvent insert @@ -241,7 +274,7 @@ describe('commitPendingOperation: link_transaction_journal_entry', () => { data: makeInvoice({ id: INV_UUID, status: 'sent', total: 1000, remaining_amount: 1000 }), error: null, }) - enqueue({ data: null, error: null }) // tx UPDATE succeeds + enqueue({ data: [{ id: TX_UUID }], error: null }) // tx UPDATE succeeds enqueue({ data: [], error: null }) // optimistic invoice UPDATE returns 0 rows enqueue({ data: null, error: null }) // compensating rollback restores tx enqueue({ data: null, error: null }) // dispatcher's reject update diff --git a/lib/reconciliation/__tests__/bank-reconciliation.test.ts b/lib/reconciliation/__tests__/bank-reconciliation.test.ts index b71d3ff9..60461fd9 100644 --- a/lib/reconciliation/__tests__/bank-reconciliation.test.ts +++ b/lib/reconciliation/__tests__/bank-reconciliation.test.ts @@ -709,12 +709,13 @@ describe('manualLink', () => { expect(result.error).toBe('Transaktionen kunde inte hittas.') }) - it('rejects when transaction is already linked', async () => { + it('rejects when transaction is already linked to a LIVE (posted) entry', async () => { const { supabase, enqueue } = createQueueMockSupabase() const tx = makeTransaction({ id: 'tx-1', journal_entry_id: 'je-existing' }) - // Transaction found but already linked + // Transaction found, still pointing at a live posted verifikat enqueue({ data: tx }) + enqueue({ data: { status: 'posted' } }) // hasLiveJournalEntryLink: prior link is live const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1') @@ -722,6 +723,24 @@ describe('manualLink', () => { expect(result.error).toBe('Transaktionen är redan kopplad till en verifikation.') }) + it('re-links a transaction stranded on a reversed entry (#988)', async () => { + const { supabase, enqueue } = createQueueMockSupabase() + // Still carries a pointer at a status='reversed' entry (storno/correction + // left it behind). The UI shows it as "utan koppling", so manualLink must + // treat it as free and overwrite the stale pointer. + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: 'je-reversed' }) + + enqueue({ data: tx }) // tx fetch + enqueue({ data: { status: 'reversed' } }) // hasLiveJournalEntryLink: stale link + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) // target JE fetch + enqueue({ data: [{ debit_amount: 1000, credit_amount: 0, account_number: '1930' }] }) // line on 1930 + enqueue({ data: [{ id: 'tx-1' }] }) // UPDATE .eq(stale id) → 1 row overwritten + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1930') + + expect(result.success).toBe(true) + }) + it('rejects when journal entry has no line on the selected account', async () => { const { supabase, enqueue } = createQueueMockSupabase() const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null }) diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index 872092e5..5863269b 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -4,6 +4,7 @@ import { eventBus } from '@/lib/events/bus' import { logMatchEvent } from '@/lib/invoices/match-log' import { fetchAllRows } from '@/lib/supabase/fetch-all' import { fetchEntryLines, type EntryLinesQuery } from '@/lib/bookkeeping/entry-lines' +import { hasLiveJournalEntryLink } from '@/lib/transactions/link-journal-entry' // ============================================================ // Types @@ -620,7 +621,11 @@ export async function manualLink( return { success: false, error: 'Transaktionen kunde inte hittas.' } } - if (tx.journal_entry_id) { + // Only a LIVE (posted) pointer blocks re-linking. A transaction still pointing + // at a 'reversed' entry (storno/correction left the link behind) reads as + // "utan koppling" in the UI, so it must be re-linkable to another verifikat + // (issue #988). The stale pointer is overwritten by the locked UPDATE below. + if (tx.journal_entry_id && (await hasLiveJournalEntryLink(supabase, companyId, tx.journal_entry_id))) { return { success: false, error: 'Transaktionen är redan kopplad till en verifikation.' } } @@ -683,11 +688,14 @@ export async function manualLink( // already-matched voucher when the user opts in via "Visa även matchade // verifikationer", so this can't happen by accident. - // Apply link. The .is('journal_entry_id', null) guard re-checks the "not - // already linked" precondition inside the write itself: the read above is - // advisory, and two concurrent linkers would otherwise silently re-point the - // row (same optimistic-lock pattern as lib/transactions/link-journal-entry.ts). - const { data: updatedRows, error: updateError } = await supabase + // Apply link. The write re-checks the pointer we validated inside the write + // itself (the read above is advisory): null for a free row, or the exact + // stale 'reversed'-entry id we're detaching from. Locking on the known value + // lets the stale-pointer overwrite through while a concurrent re-link becomes + // a no-op (0 rows → the "redan kopplad" branch below). Same optimistic-lock + // pattern as lib/transactions/link-journal-entry.ts. + const previousJournalEntryId = (tx.journal_entry_id as string | null) ?? null + const linkUpdate = supabase .from('transactions') .update({ journal_entry_id: journalEntryId, @@ -696,8 +704,10 @@ export async function manualLink( }) .eq('id', transactionId) .eq('company_id', companyId) - .is('journal_entry_id', null) - .select('id') + const { data: updatedRows, error: updateError } = await (previousJournalEntryId === null + ? linkUpdate.is('journal_entry_id', null) + : linkUpdate.eq('journal_entry_id', previousJournalEntryId) + ).select('id') if (updateError) { return { success: false, error: 'Kunde inte koppla transaktionen. Försök igen.' } diff --git a/lib/transactions/__tests__/link-journal-entry.test.ts b/lib/transactions/__tests__/link-journal-entry.test.ts new file mode 100644 index 00000000..6d4e984d --- /dev/null +++ b/lib/transactions/__tests__/link-journal-entry.test.ts @@ -0,0 +1,54 @@ +/** + * Unit tests for hasLiveJournalEntryLink. + * + * This is the predicate the re-booking guards (linkTransactionToJournalEntry, + * manualLink, categorize-core, the MCP stage-check) share to decide whether a + * transaction's journal_entry_id is a LIVE link that should block re-linking, + * or a stale pointer at a reversed/cancelled entry that the UI already shows as + * "utan koppling" and must stay re-linkable (issue #988). + * + * The end-to-end re-link behaviour is covered by the route test + * (app/api/transactions/[id]/link-journal-entry/__tests__/route.test.ts) and + * the pending-op commit test. + */ +import { describe, it, expect } from 'vitest' +import { createQueuedMockSupabase } from '@/tests/helpers' +import { hasLiveJournalEntryLink } from '../link-journal-entry' + +describe('hasLiveJournalEntryLink', () => { + it('returns false for a null/undefined pointer without querying', async () => { + const { supabase } = createQueuedMockSupabase() + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', null)).toBe(false) + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', undefined)).toBe(false) + }) + + it('returns true when the entry is posted (a live link)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { status: 'posted' }, error: null }) + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', 'je-1')).toBe(true) + }) + + it('returns false when the entry is reversed (stale link, #988)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { status: 'reversed' }, error: null }) + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', 'je-1')).toBe(false) + }) + + it('returns false when the entry is cancelled', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { status: 'cancelled' }, error: null }) + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', 'je-1')).toBe(false) + }) + + it('returns false when the referenced entry row is missing', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: null }) + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', 'je-gone')).toBe(false) + }) + + it('fails closed (returns true) on a read error so a live link is never clobbered', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: { message: 'statement timeout' } }) + expect(await hasLiveJournalEntryLink(supabase as never, 'company-1', 'je-1')).toBe(true) + }) +}) diff --git a/lib/transactions/categorize-core.ts b/lib/transactions/categorize-core.ts index 3cd4c6ba..10400836 100644 --- a/lib/transactions/categorize-core.ts +++ b/lib/transactions/categorize-core.ts @@ -29,6 +29,7 @@ import { upsertCounterpartyTemplate } from '@/lib/bookkeeping/counterparty-templ import { isBookkeepingError } from '@/lib/bookkeeping/errors' import { linkToJournalEntry } from '@/lib/core/documents/document-service' import { detectBookingDuplicate, type BookingDuplicateExclusions } from '@/lib/transactions/booking-duplicate-detection' +import { hasLiveJournalEntryLink } from '@/lib/transactions/link-journal-entry' import { appendProcessingHistory } from '@/lib/processing-history/append' import { roundOre } from '@/lib/money' import { createLogger } from '@/lib/logger' @@ -165,7 +166,17 @@ export async function categorizeMatchedTransaction( if (fetchError || !transaction) { return { error: 'Transaction not found: it may have been deleted.', status: 404 } } - if (transaction.journal_entry_id) { + // A stale pointer at a 'reversed' entry (storno/correction left it behind) + // must not block re-categorization: the row reads as "utan koppling" in the + // UI, so a fresh booking has to be allowed (issue #988). Only a live posted + // link means it was genuinely categorized in the meantime. The UPDATE below + // is unconditional (no null-lock), so it overwrites the stale pointer; the + // duplicate guard still catches an existing live correction and steers the + // user to link instead. + if ( + transaction.journal_entry_id && + (await hasLiveJournalEntryLink(supabase, companyId, transaction.journal_entry_id)) + ) { return { error: 'Transaction already has a journal entry: it was categorized in the meantime.', status: 409 } } diff --git a/lib/transactions/link-journal-entry.ts b/lib/transactions/link-journal-entry.ts index 571fd70b..8cc1f325 100644 --- a/lib/transactions/link-journal-entry.ts +++ b/lib/transactions/link-journal-entry.ts @@ -79,6 +79,43 @@ export function formatVoucherLabel( return num === '' ? series : `${series}-${num}` } +/** + * Is `journalEntryId` a LIVE link, i.e. does it reference a posted verifikat? + * + * A transaction can carry a non-null `journal_entry_id` that no longer points + * at a live booking: reversing (storno) or correcting an entry marks the + * original `reversed`, and while both flows try to detach or re-point the + * transaction (engine.ts `reverseEntry`, storno-service `relinkTransactions- + * ToEntry`), those re-links are best-effort and rows reversed before #726 + * (2026-06-15) were never touched at all. Such a transaction reads as "utan + * koppling" in the UI: the transactions page enriches only `status='posted'` + * links, so a reversed pointer renders as no link, yet the raw column is still + * set. + * + * The "already linked" guards on the re-booking paths must mirror that same + * posted-only predicate. If they treat any non-null pointer as linked, a + * transaction the UI shows as free can never be re-linked or re-categorized + * (issue #988). Returns true ONLY when the pointer references a posted entry; + * null / missing / reversed / cancelled / draft all count as no live link, so + * the caller may overwrite the stale pointer. Fails closed (returns true) on a + * read error so a transient lookup blip can never detach a genuinely live link. + */ +export async function hasLiveJournalEntryLink( + supabase: SupabaseClient, + companyId: string, + journalEntryId: string | null | undefined, +): Promise { + if (!journalEntryId) return false + const { data, error } = await supabase + .from('journal_entries') + .select('status') + .eq('id', journalEntryId) + .eq('company_id', companyId) + .maybeSingle() + if (error) return true + return data?.status === 'posted' +} + export async function linkTransactionToJournalEntry( supabase: SupabaseClient, userId: string, @@ -103,7 +140,15 @@ export async function linkTransactionToJournalEntry( return { ok: false, code: 'TX_CATEGORIZE_TX_NOT_FOUND' } } - if (transaction.journal_entry_id) { + // Only a LIVE (posted) pointer blocks re-linking. A pointer left behind by a + // storno/correction references a 'reversed' entry: the UI already shows the + // row as "utan koppling", so the guard must agree and let the user re-link it + // to another verifikat (issue #988). The stale pointer is overwritten by the + // optimistic-locked UPDATE below. + if ( + transaction.journal_entry_id && + (await hasLiveJournalEntryLink(supabase, companyId, transaction.journal_entry_id as string)) + ) { return { ok: false, code: 'LINK_TX_TX_ALREADY_LINKED', @@ -211,14 +256,20 @@ export async function linkTransactionToJournalEntry( // if a subsequent step fails: otherwise a partial state would persist // (tx linked, invoice unchanged, no payment row). const priorTxState = { - journal_entry_id: transaction.journal_entry_id, // validated null above + // null, or a stale 'reversed'-entry id we're clearing (validated not-live above) + journal_entry_id: transaction.journal_entry_id, invoice_id: transaction.invoice_id, potential_invoice_id: transaction.potential_invoice_id, potential_supplier_invoice_id: transaction.potential_supplier_invoice_id, is_business: transaction.is_business, } - const { error: updateTxError } = await supabase + // Optimistic lock on the pointer we validated: null for a free row, or the + // exact stale id for one we're detaching from a reversed entry. Locking on + // the known value (rather than always .is(null)) lets the stale-pointer + // overwrite through while still turning a concurrent re-link into a no-op. + const previousJournalEntryId = (transaction.journal_entry_id as string | null) ?? null + const txUpdate = supabase .from('transactions') .update({ journal_entry_id: journalEntryId, @@ -229,11 +280,26 @@ export async function linkTransactionToJournalEntry( }) .eq('id', transactionId) .eq('company_id', companyId) - .is('journal_entry_id', null) + const { data: updatedTxRows, error: updateTxError } = await (previousJournalEntryId === null + ? txUpdate.is('journal_entry_id', null) + : txUpdate.eq('journal_entry_id', previousJournalEntryId) + ).select('id') if (updateTxError) { return { ok: false, code: 'LINK_TX_DB_ERROR', details: { reason: updateTxError.message } } } + // CAS lost: a concurrent linker changed the pointer between the liveness + // check and this write, so 0 rows matched. Fail BEFORE any invoice side + // effects: otherwise we'd settle the invoice + insert an invoice_payments row + // for a transaction we didn't actually link (same optimistic-lock contract as + // manualLink in lib/reconciliation/bank-reconciliation.ts). + if (!updatedTxRows || updatedTxRows.length === 0) { + return { + ok: false, + code: 'LINK_TX_TX_ALREADY_LINKED', + details: { existingJournalEntryId: previousJournalEntryId }, + } + } async function rollbackTxLink(reason: string): Promise { // SOC 2 PI1.3 (processing integrity): if a rollback itself fails, the