fix(pending-ops): record posted ids and land failed_partial instead of clean rejected after partial commits (#842) (#1110)

Multi-step executors (match_transaction_invoice, credit_invoice) post an
irreversible voucher or persist a credit note and then run later fallible
steps. A failure there previously marked the whole op status=rejected,
hiding the posted entity and its id from operators.

- new migration 20260722134114: add failed_partial to the
  pending_operations status CHECK and treat it as terminal in both
  immutability triggers (immutable, undeletable, never re-claimable)
- PartialCommitError + ExecutorResult.partialPostedIds carry the posted
  ids; the dispatcher writes status=failed_partial with
  result_data.posted_ids and returns code=partial_commit
- instrument only the two named executors; hoist the read-only
  settlement-account resolution above the storno in the match executor
- consumer sweep: status union + query schema widened, failed_partial
  folds into the Avvisade tab with a badge and posted-ids detail line,
  bulk/reject routes and MCP tools message it explicitly, worklist and
  expiry sweep intentionally untouched (not pending work)
- tests: pg-real coverage for the new terminal semantics, dispatcher unit
  tests for both partial paths plus byte-for-byte regression guards

Fixes #842

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-07-22 18:33:49 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent 27ef398623
commit 3e1ea29d02
16 changed files with 795 additions and 23 deletions
+3 -1
View File
@@ -1905,7 +1905,9 @@ export const EventsQuerySchema = z.object({
// ============================================================
export const PendingOperationsQuerySchema = z.object({
status: z.enum(['pending', 'committed', 'rejected']).default('pending'),
// 'failed_partial' is queryable directly; the UI folds it into the
// rejected tab (see app/api/pending-operations/route.ts).
status: z.enum(['pending', 'committed', 'rejected', 'failed_partial']).default('pending'),
limit: z.coerce.number().int().min(1).max(100).default(50),
offset: z.coerce.number().int().nonnegative().default(0),
})
@@ -0,0 +1,343 @@
/**
* failed_partial dispatcher coverage (issue #842).
*
* Multi-step executors post an irreversible voucher (or persist a credit
* note) and then run later fallible steps. When such a later step fails, the
* dispatcher must land the op in the terminal 'failed_partial' status with
* the posted ids in result_data.posted_ids, instead of a clean-looking
* 'rejected' that hides the orphaned voucher. Clean failures (nothing posted
* yet) must keep today's 'rejected' behavior byte-for-byte.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { eventBus } from '@/lib/events/bus'
import { createQueuedMockSupabase } from '@/tests/helpers'
import { JournalEntryNotBalancedError } from '@/lib/bookkeeping/errors'
import type { PendingOperation } from '@/types'
const mockCreatePaymentEntry = vi.fn()
const mockCreateCashEntry = vi.fn()
const mockCreateCreditNoteEntry = vi.fn()
vi.mock('@/lib/bookkeeping/invoice-entries', async () => {
const actual = await vi.importActual<typeof import('@/lib/bookkeeping/invoice-entries')>(
'@/lib/bookkeeping/invoice-entries',
)
return {
...actual,
createInvoicePaymentJournalEntry: (...args: unknown[]) => mockCreatePaymentEntry(...args),
createInvoiceCashEntry: (...args: unknown[]) => mockCreateCashEntry(...args),
createCreditNoteJournalEntry: (...args: unknown[]) => mockCreateCreditNoteEntry(...args),
}
})
const mockReverseEntry = vi.fn()
vi.mock('@/lib/bookkeeping/engine', async () => {
const actual = await vi.importActual<typeof import('@/lib/bookkeeping/engine')>(
'@/lib/bookkeeping/engine',
)
return {
...actual,
reverseEntry: (...args: unknown[]) => mockReverseEntry(...args),
}
})
import { commitPendingOperation } from '../commit'
function makePendingOp(overrides: Partial<PendingOperation>): PendingOperation {
return {
id: 'op-1',
user_id: 'user-1',
company_id: 'company-1',
operation_type: 'match_transaction_invoice',
status: 'pending',
title: 'test',
params: {},
preview_data: {},
result_data: null,
actor_type: 'user',
actor_id: null,
actor_label: null,
risk_level: 'medium',
created_at: '2026-07-22T00:00:00Z',
resolved_at: null,
updated_at: '2026-07-22T00:00:00Z',
...overrides,
} as PendingOperation
}
/**
* Wraps the queued mock so every .update(payload) is recorded per table:
* the queued helper drops call arguments, but these tests must assert WHAT
* the dispatcher wrote to pending_operations, not only the returned status.
*/
function recordUpdates(supabase: { from: ReturnType<typeof vi.fn> }) {
const updates: Array<{ table: string; payload: Record<string, unknown> }> = []
const original = supabase.from.getMockImplementation() as (table: string) => unknown
supabase.from.mockImplementation((table: string) => {
const chain = original(table) as object
return new Proxy(chain, {
get(target, prop, receiver) {
if (prop === 'update') {
return (payload: Record<string, unknown>) => {
updates.push({ table, payload })
return (Reflect.get(target, 'update', receiver) as (p: unknown) => unknown)(payload)
}
}
return Reflect.get(target, prop, receiver)
},
})
})
return updates
}
function pendingOpUpdates(updates: Array<{ table: string; payload: Record<string, unknown> }>) {
return updates.filter((u) => u.table === 'pending_operations')
}
const baseTransaction = {
id: 'tx-1',
company_id: 'company-1',
amount: 500,
currency: 'SEK',
date: '2026-05-12',
invoice_id: null,
journal_entry_id: null,
cash_account_id: null,
}
const baseInvoice = {
id: 'inv-1',
invoice_number: 'F-2026001',
status: 'sent',
total: 500,
remaining_amount: 500,
paid_amount: 0,
currency: 'SEK',
exchange_rate: null,
journal_entry_id: null,
credited_invoice_id: null,
customer: { name: 'Kund AB' },
}
beforeEach(() => {
vi.clearAllMocks()
eventBus.clear()
mockCreatePaymentEntry.mockResolvedValue({ id: 'je-pay' })
mockCreateCashEntry.mockResolvedValue({ id: 'je-pay' })
mockCreateCreditNoteEntry.mockResolvedValue({ id: 'je-credit' })
mockReverseEntry.mockResolvedValue({ id: 'je-storno' })
})
describe('match_transaction_invoice: partial commit after the storno', () => {
it('lands failed_partial with the reversal voucher id when the payment JE throws after the storno', async () => {
const { supabase, enqueue } = createQueuedMockSupabase()
const updates = recordUpdates(supabase)
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: { ...baseTransaction, journal_entry_id: 'je-old' }, error: null }) // transaction fetch
enqueue({ data: baseInvoice, error: null }) // invoice fetch
enqueue({ data: { accounting_method: 'accrual', entity_type: 'aktiebolag' }, error: null }) // settings
enqueue({ data: null, error: null }) // transactions unlink after storno
enqueue({ data: null, error: null }) // dispatcher pending_operations update
mockCreatePaymentEntry.mockRejectedValue(new JournalEntryNotBalancedError(500, 400))
const op = makePendingOp({ params: { transaction_id: 'tx-1', invoice_id: 'inv-1' } })
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(mockReverseEntry).toHaveBeenCalledWith(expect.anything(), 'company-1', 'user-1', 'je-old')
expect(result.status).toBe('failed')
expect(result.http_status).toBe(500)
expect(result.code).toBe('partial_commit')
expect(result.data).toEqual({ posted_ids: { reversal_journal_entry_id: 'je-storno' } })
const opUpdates = pendingOpUpdates(updates)
// First write is the atomic claim, second is the terminal status.
expect(opUpdates[0]?.payload).toEqual({ status: 'committing' })
expect(opUpdates[1]?.payload).toMatchObject({
status: 'failed_partial',
result_data: {
threw: true,
posted_ids: { reversal_journal_entry_id: 'je-storno' },
},
})
expect(opUpdates[1]?.payload.resolved_at).toBeTruthy()
})
it('lands failed_partial with the payment JE id when the invoice CAS update matches zero rows', async () => {
const { supabase, enqueue } = createQueuedMockSupabase()
const updates = recordUpdates(supabase)
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: baseTransaction, error: null }) // transaction fetch (no prior JE: no storno)
enqueue({ data: baseInvoice, error: null }) // invoice fetch
enqueue({ data: { accounting_method: 'accrual', entity_type: 'aktiebolag' }, error: null }) // settings
enqueue({ data: [], error: null }) // invoice CAS update: zero rows (raced fully-paid)
enqueue({ data: null, error: null }) // dispatcher pending_operations update
const op = makePendingOp({ params: { transaction_id: 'tx-1', invoice_id: 'inv-1' } })
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
// A 409 AFTER the payment voucher posted must NOT auto-reject: it is a
// partial commit and the posted JE id must be surfaced.
expect(result.status).toBe('failed')
expect(result.http_status).toBe(409)
expect(result.auto_rejected).toBeUndefined()
expect(result.code).toBe('partial_commit')
expect(result.data).toEqual({ posted_ids: { payment_journal_entry_id: 'je-pay' } })
const opUpdates = pendingOpUpdates(updates)
expect(opUpdates[1]?.payload).toMatchObject({
status: 'failed_partial',
result_data: {
http_status: 409,
posted_ids: { payment_journal_entry_id: 'je-pay' },
},
})
})
it('keeps the clean rejected path when the failure happens BEFORE anything is posted', async () => {
// The settlement-account lookup now runs before the storno: an infra
// failure there must reject the op with nothing posted and must NOT be
// labeled a partial commit.
const { supabase, enqueue } = createQueuedMockSupabase()
const updates = recordUpdates(supabase)
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({
data: { ...baseTransaction, journal_entry_id: 'je-old', cash_account_id: 'ca-broken' },
error: null,
}) // transaction fetch
enqueue({ data: baseInvoice, error: null }) // invoice fetch
enqueue({ data: { accounting_method: 'accrual', entity_type: 'aktiebolag' }, error: null }) // settings
enqueue({ data: null, error: { message: 'connection reset' } }) // cash_accounts lookup errors
enqueue({ data: null, error: null }) // dispatcher pending_operations update
const op = makePendingOp({ params: { transaction_id: 'tx-1', invoice_id: 'inv-1' } })
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(mockReverseEntry).not.toHaveBeenCalled()
expect(result.status).toBe('failed')
expect(result.code).toBeUndefined()
const opUpdates = pendingOpUpdates(updates)
expect(opUpdates[1]?.payload).toMatchObject({
status: 'rejected',
result_data: { threw: true },
})
})
it('keeps the 409 auto-reject when the CAS update races and no voucher was posted', async () => {
const { supabase, enqueue } = createQueuedMockSupabase()
const updates = recordUpdates(supabase)
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: baseTransaction, error: null }) // transaction fetch
enqueue({ data: baseInvoice, error: null }) // invoice fetch
enqueue({ data: { accounting_method: 'accrual', entity_type: 'aktiebolag' }, error: null }) // settings
enqueue({ data: [], error: null }) // invoice CAS update: zero rows
enqueue({ data: null, error: null }) // dispatcher pending_operations update
// JE creation "succeeded" with null (e.g. no open fiscal period):
// nothing was posted, so today's auto-reject semantics must survive.
mockCreatePaymentEntry.mockResolvedValue(null)
const op = makePendingOp({ params: { transaction_id: 'tx-1', invoice_id: 'inv-1' } })
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(result.status).toBe('rejected')
expect(result.auto_rejected).toBe(true)
expect(result.http_status).toBe(409)
const opUpdates = pendingOpUpdates(updates)
expect(opUpdates[1]?.payload).toMatchObject({
status: 'rejected',
result_data: { auto_rejected: true },
})
})
})
describe('credit_invoice: partial commit after the credit note persisted', () => {
const originalInvoice = {
id: 'inv-1',
invoice_number: 'F-2026001',
status: 'sent',
document_type: 'invoice',
customer_id: 'cust-1',
delivery_date: null,
currency: 'SEK',
exchange_rate: null,
exchange_rate_date: null,
subtotal: 400,
subtotal_sek: 400,
vat_amount: 100,
vat_amount_sek: 100,
total: 500,
total_sek: 500,
vat_treatment: 'standard_25',
vat_rate: 25,
moms_ruta: null,
reverse_charge_text: null,
your_reference: null,
our_reference: null,
journal_entry_id: null,
default_dimensions: {},
items: [],
}
it('lands failed_partial with the credit note id when the credit-note JE throws', async () => {
const { supabase, enqueue } = createQueuedMockSupabase()
const updates = recordUpdates(supabase)
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: originalInvoice, error: null }) // original invoice fetch
enqueue({ data: { id: 'cn-1', invoice_date: '2026-07-22' }, error: null }) // credit note insert
enqueue({ data: null, error: null }) // invoice_items insert
enqueue({ data: null, error: null }) // original invoice -> credited
enqueue({
data: { id: 'cn-1', invoice_date: '2026-07-22', items: [], customer: { name: 'Kund AB' } },
error: null,
}) // complete credit note fetch
enqueue({ data: { entity_type: 'aktiebolag', accounting_method: 'accrual' }, error: null }) // settings
enqueue({ data: null, error: null }) // dispatcher pending_operations update
mockCreateCreditNoteEntry.mockRejectedValue(new JournalEntryNotBalancedError(500, 400))
const op = makePendingOp({
operation_type: 'credit_invoice',
params: { invoice_id: 'inv-1' },
})
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(result.status).toBe('failed')
expect(result.code).toBe('partial_commit')
expect(result.data).toEqual({
posted_ids: { credit_note_id: 'cn-1', original_invoice_id: 'inv-1' },
})
const opUpdates = pendingOpUpdates(updates)
expect(opUpdates[1]?.payload).toMatchObject({
status: 'failed_partial',
result_data: {
threw: true,
posted_ids: { credit_note_id: 'cn-1', original_invoice_id: 'inv-1' },
},
})
})
it('keeps the clean rejected path when the credit note itself fails to persist', async () => {
const { supabase, enqueue } = createQueuedMockSupabase()
const updates = recordUpdates(supabase)
enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim
enqueue({ data: originalInvoice, error: null }) // original invoice fetch
enqueue({ data: null, error: { message: 'insert failed' } }) // credit note insert fails
enqueue({ data: null, error: null }) // dispatcher pending_operations update
const op = makePendingOp({
operation_type: 'credit_invoice',
params: { invoice_id: 'inv-1' },
})
const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op)
expect(result.status).toBe('failed')
expect(result.code).toBeUndefined()
expect(mockCreateCreditNoteEntry).not.toHaveBeenCalled()
const opUpdates = pendingOpUpdates(updates)
expect(opUpdates[1]?.payload).toMatchObject({ status: 'rejected' })
})
})
+124 -10
View File
@@ -60,6 +60,7 @@ import {
type SkatteverketCommitServices,
type SkvSubmitResult,
} from '@/lib/pending-operations/skatteverket-commit'
import { PartialCommitError } from '@/lib/pending-operations/errors'
import { getEmailService } from '@/lib/email/service'
import { hasCapability, CAPABILITY_BLOCKED_MESSAGE_SV } from '@/lib/entitlements/has-capability'
import { PAID_OPERATION_CAPABILITY_MAP } from '@/lib/entitlements/keys'
@@ -192,7 +193,16 @@ async function recordSkippedInvoiceJournalEntry(
// ── Executors ────────────────────────────────────────────────────
type ExecutorResult = { data?: Record<string, unknown>; error?: string; status?: number }
type ExecutorResult = {
data?: Record<string, unknown>
error?: string
status?: number
// Set when the executor already performed an irreversible side-effect
// (posted voucher, persisted credit note) before the failure in `error`:
// the dispatcher then lands the op in 'failed_partial' instead of
// 'rejected' and persists these ids in result_data.posted_ids (issue #842).
partialPostedIds?: Record<string, string>
}
async function commitCategorizeTransaction(
supabase: SupabaseClient,
@@ -1722,13 +1732,13 @@ async function commitMatchTransactionInvoice(
}
const { newPaidAmount, newRemaining, isFullyPaid, newStatus } = payment.plan
if (transaction.journal_entry_id) {
await reverseEntry(supabase, companyId, userId, transaction.journal_entry_id)
await supabase.from('transactions').update({ journal_entry_id: null }).eq('id', transactionId)
}
const now = new Date().toISOString()
// Read-only prevalidation, deliberately hoisted ABOVE the irreversible
// storno below (issue #842): resolveSettlementAccount can throw
// (BookkeepingDatabaseError on a failed cash_accounts lookup), and a throw
// here must reject the op with NOTHING posted. Behavior-preserving on the
// happy path: these are pure reads.
const { data: settings } = await supabase
.from('company_settings').select('accounting_method, entity_type').eq('company_id', companyId).single()
@@ -1746,6 +1756,17 @@ async function commitMatchTransactionInvoice(
// transaction settled into. Mirrors the match-invoice route fix.
const paymentAccount = await resolveSettlementAccount(supabase, companyId, transaction.cash_account_id, log)
// From here on the executor posts irreversible vouchers. Track their ids so
// a later failure can land the op in 'failed_partial' carrying them
// (issue #842) instead of a clean-looking 'rejected'.
const postedIds: Record<string, string> = {}
if (transaction.journal_entry_id) {
const reversal = await reverseEntry(supabase, companyId, userId, transaction.journal_entry_id)
postedIds.reversal_journal_entry_id = reversal.id
await supabase.from('transactions').update({ journal_entry_id: null }).eq('id', transactionId)
}
let journalEntryId: string | null = null
try {
if (useCashEntry) {
@@ -1762,9 +1783,26 @@ async function commitMatchTransactionInvoice(
journalEntryId = je?.id ?? null
}
} catch (err) {
if (isBookkeepingError(err)) throw err
// Recoverable: the dispatcher releases the op back to 'pending' and this
// executor is re-entrant past the storno (the transaction was unlinked
// above, so a retry does not post a second storno). Keep that path even
// when a reversal voucher was already posted.
if (err instanceof AccountsNotInChartError) throw err
if (isBookkeepingError(err)) {
// A reversal voucher already posted makes this a partial commit, not a
// clean failure: surface the storno id (issue #842).
if (Object.keys(postedIds).length > 0) {
throw new PartialCommitError(
`match_transaction_invoice failed after posting a reversal voucher: ${err instanceof Error ? err.message : 'journal entry creation failed'}`,
postedIds,
err,
)
}
throw err
}
log.error('Failed to create match journal entry:', err)
}
if (journalEntryId) postedIds.payment_journal_entry_id = journalEntryId
const { data: updatedRows, error: updateInvError } = await supabase
.from('invoices')
@@ -1778,9 +1816,19 @@ async function commitMatchTransactionInvoice(
.in('status', ['sent', 'overdue', 'partially_paid'])
.select('id')
if (updateInvError) return { error: 'Failed to update invoice status', status: 500 }
if (updateInvError) {
return {
error: 'Failed to update invoice status',
status: 500,
...(Object.keys(postedIds).length > 0 ? { partialPostedIds: postedIds } : {}),
}
}
if (!updatedRows || updatedRows.length === 0) {
return { error: 'Invoice has already been fully paid or is no longer matchable', status: 409 }
return {
error: 'Invoice has already been fully paid or is no longer matchable',
status: 409,
...(Object.keys(postedIds).length > 0 ? { partialPostedIds: postedIds } : {}),
}
}
const paymentNotes = (accountingMethod === 'cash' && !isFullyPaid)
@@ -3034,7 +3082,19 @@ async function commitCreditInvoice(
.eq('id', creditNote.id)
}
} catch (err) {
if (isBookkeepingError(err)) throw err
if (isBookkeepingError(err)) {
// The credit note row and the original's 'credited' flip are already
// persisted: a clean 'rejected' would hide them. Land the op in
// 'failed_partial' carrying the ids (issue #842). This intentionally
// covers AccountsNotInChartError too: the release-to-pending retry
// path cannot recover a credit_invoice op (re-running the executor
// auto-rejects with 409 because the original is already 'credited').
throw new PartialCommitError(
`credit_invoice failed after persisting the credit note: ${err instanceof Error ? err.message : 'journal entry creation failed'}`,
{ credit_note_id: creditNote.id, original_invoice_id: id },
err,
)
}
log.error('Failed to create credit note journal entry:', err)
}
@@ -4615,6 +4675,30 @@ async function commitPendingOperationInner(
}
}
} catch (err) {
// Partial commit (issue #842): the executor already posted an
// irreversible side-effect (storno voucher, credit note) before a later
// step failed. 'rejected' would misrepresent reality and hide the posted
// entity, so land the op in the terminal 'failed_partial' status with the
// posted ids in result_data so an operator can locate the orphan. Checked
// FIRST: a wrapped recoverable cause must NOT release the claim back to
// 'pending' (the side-effect already exists).
if (err instanceof PartialCommitError) {
await supabase
.from('pending_operations')
.update({
status: 'failed_partial',
resolved_at: new Date().toISOString(),
result_data: { error: err.message, threw: true, posted_ids: err.postedIds },
})
.eq('id', pendingOp.id)
return {
status: 'failed',
error: err.message,
http_status: 500,
code: 'partial_commit',
data: { posted_ids: err.postedIds },
}
}
// Accounts-not-in-chart is RECOVERABLE: the booking itself is valid; the
// company's chart just lacks the (standard BAS) accounts it posts to. Do
// NOT consume the op: release the atomic claim back to 'pending' so the
@@ -4670,6 +4754,36 @@ async function commitPendingOperationInner(
}
if (result.error) {
// Structured partial marker (issue #842): same semantics as the
// PartialCommitError branch above, for executors that report the failure
// via the ExecutorResult contract instead of throwing. Must run before
// the auto-reject branch: a 409 AFTER a voucher was posted is a partial
// commit, not a re-stageable rejection.
const partialPostedIds =
result.partialPostedIds && Object.keys(result.partialPostedIds).length > 0
? result.partialPostedIds
: null
if (partialPostedIds) {
await supabase
.from('pending_operations')
.update({
status: 'failed_partial',
resolved_at: new Date().toISOString(),
result_data: {
error: result.error,
http_status: result.status,
posted_ids: partialPostedIds,
},
})
.eq('id', pendingOp.id)
return {
status: 'failed',
error: result.error,
http_status: result.status ?? 500,
code: 'partial_commit',
data: { posted_ids: partialPostedIds },
}
}
const isAutoReject = result.status === 404 || result.status === 409
await supabase
.from('pending_operations')
+35
View File
@@ -0,0 +1,35 @@
/**
* Typed errors for the pending-operations commit path.
*
* PartialCommitError is thrown by multi-step executors when an irreversible
* side-effect (posted voucher, persisted credit note) already exists and a
* LATER step fails. The dispatcher (commitPendingOperationInner) detects it
* and lands the op in the terminal status 'failed_partial' instead of
* 'rejected', persisting the posted ids in result_data.posted_ids so an
* operator can locate the orphaned voucher/entity (issue #842).
*
* Do NOT throw this before the first irreversible step: a clean failure with
* nothing posted must keep today's 'rejected' semantics.
*/
export class PartialCommitError extends Error {
readonly name = 'PartialCommitError'
/**
* Ids of the entities that were irreversibly created before the failure,
* keyed by a stable snake_case label (e.g. reversal_journal_entry_id,
* payment_journal_entry_id, credit_note_id). Persisted verbatim into
* pending_operations.result_data.posted_ids.
*/
readonly postedIds: Record<string, string>
/** The underlying failure that interrupted the executor. */
readonly cause: unknown
constructor(message: string, postedIds: Record<string, string>, cause?: unknown) {
super(message)
this.postedIds = postedIds
this.cause = cause
}
}
export function isPartialCommitError(err: unknown): err is PartialCommitError {
return err instanceof PartialCommitError
}