From 96223825791f1cae323bc0678d95974b3651870d Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Sun, 23 Aug 2026 01:57:24 +0200 Subject: [PATCH] fix(mcp): surface the database reason and code behind LINK_TX_DB_ERROR to the approver (#1807) * fix(mcp): surface the database reason and code behind LINK_TX_DB_ERROR to the approver gnubok_link_transaction_to_journal_entry failed reproducibly for a customer on certain incoming payments with a bare LINK_TX_DB_ERROR: the service put the Postgres message in details.reason, but the code had no structured entry and the commit dispatcher dropped executor data on failure, so neither the MCP approve result nor result_data said why. LINK_TX_DB_ERROR now has a structured entry; the executor appends the DB reason to the message and sets errorCode; the dispatcher persists and returns executor failure details (result_data.details, CommitResult.data, .code); gnubok_approve_pending_operation exposes error_code. The next failing call tells us which constraint or trigger fired. Co-Authored-By: Claude Fable 5 * fix(mcp): keep tools/list under the context budget (drop approve schema descriptions) The two output-schema descriptions added for data/error_code pushed the projected tools/list payload 7 tokens over the ceiling guarded by payload-size.bench.test.ts. The fields stay; the prose goes. Co-Authored-By: Claude Fable 5 * fix(pending-ops): log loudly when the terminal rejected write fails Review finding: the rejection branch wrote pending_operations without checking the result, so a failed write left the row in 'committing' with the executor error, code and details lost silently. Mirror the finalize branch: inspect the write result and log with the ids plus the failure we could not persist; the daily recovery sweep still resolves the row. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- extensions/general/mcp-server/server.ts | 2 + .../__tests__/structured-errors.test.ts | 19 +++++++++ lib/errors/structured-errors.ts | 10 +++++ .../link-transaction-journal-entry.test.ts | 38 +++++++++++++++++ lib/pending-operations/commit.ts | 42 +++++++++++++++++-- 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 0a0e2744..bd0e26c8 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -16246,6 +16246,7 @@ export const tools: McpTool[] = [ operation_id: { type: 'string' }, data: { type: 'object' }, error: { type: 'string' }, + error_code: { type: 'string' }, auto_rejected: { type: 'boolean' }, }, required: ['status', 'operation_id'], @@ -16358,6 +16359,7 @@ export const tools: McpTool[] = [ operation_id: operationId, ...(result.data ? { data: result.data } : {}), ...(result.error ? { error: result.error } : {}), + ...(result.code ? { error_code: result.code } : {}), ...(result.auto_rejected ? { auto_rejected: true } : {}), } }, diff --git a/lib/errors/__tests__/structured-errors.test.ts b/lib/errors/__tests__/structured-errors.test.ts index ba6a7843..007713c6 100644 --- a/lib/errors/__tests__/structured-errors.test.ts +++ b/lib/errors/__tests__/structured-errors.test.ts @@ -39,6 +39,25 @@ describe('structured-errors registry', () => { } }) + it('has an entry for every code the link-transaction service can emit', () => { + for (const code of [ + 'LINK_TX_JE_NOT_FOUND', + 'LINK_TX_JE_NOT_POSTED', + 'LINK_TX_TX_ALREADY_LINKED', + 'LINK_TX_INVOICE_NOT_FOUND', + 'LINK_TX_INVOICE_NOT_OPEN', + 'LINK_TX_INVOICE_CREDIT_NOTE', + 'LINK_TX_INVOICE_RACE', + 'LINK_TX_INVOICE_CURRENCY_MISMATCH', + 'LINK_TX_DB_ERROR', + ]) { + const entry = getErrorEntry(code) + expect(entry, `missing entry for ${code}`).toBeDefined() + expect(entry?.message_sv).toBeTruthy() + expect(entry?.message_en).toBeTruthy() + } + }) + it('listErrorCodes returns at least the bookkeeping + generic + provider codes', () => { const codes = listErrorCodes() expect(codes.length).toBeGreaterThan(20) diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index 6c33328c..e9d90da9 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -678,6 +678,16 @@ const LINK_TX_JE: Record = { message_en: 'Transaction and invoice currency must match to link to an existing voucher. Use the match-invoice flow for cross-currency settlement.', }, + // Raw database failure on the transaction or invoice UPDATE. The service + // puts the Postgres message in details.reason; callers append it so the + // constraint or trigger that fired is visible to the agent instead of a + // bare code (a customer hit this reproducibly on certain positive amounts + // and could not tell us why). + LINK_TX_DB_ERROR: { + httpStatus: 500, + message_sv: 'Kopplingen kunde inte sparas i databasen.', + message_en: 'Linking the transaction to the journal entry failed at the database.', + }, } const MATCH_SI: Record = { 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 7f589b97..c7f19d3b 100644 --- a/lib/pending-operations/__tests__/link-transaction-journal-entry.test.ts +++ b/lib/pending-operations/__tests__/link-transaction-journal-entry.test.ts @@ -296,6 +296,44 @@ describe('commitPendingOperation: link_transaction_journal_entry', () => { expect(result.error).toBe('Credit notes cannot be recorded as paid.') }) + it('surfaces the database reason and code when the transaction UPDATE fails (LINK_TX_DB_ERROR)', async () => { + // A customer hit LINK_TX_DB_ERROR reproducibly on certain incoming + // payments and could not tell us why: the Postgres message was logged + // but dropped on the way to the approver. The dispatcher must carry the + // reason, the structured code and the details through. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ + data: makeTransaction({ id: TX_UUID, journal_entry_id: null, amount: 313, date: '2026-05-15' }), + error: null, + }) + enqueue({ + data: { + id: JE_UUID, + status: 'posted', + voucher_series: 'A', + voucher_number: 12, + entry_date: '2026-05-15', + }, + error: null, + }) + const pgMessage = + 'new row for relation "transactions" violates check constraint "transactions_is_ignored_no_journal_entry"' + enqueue({ data: null, error: { message: pgMessage, code: '23514' } }) // tx UPDATE fails + enqueue({ data: null, error: null }) // dispatcher's reject/fail 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('failed') + expect(result.http_status).toBe(500) + expect(result.code).toBe('LINK_TX_DB_ERROR') + expect(result.error).toContain(pgMessage) + expect(result.data).toMatchObject({ reason: pgMessage }) + }) + it('returns 409 LINK_TX_INVOICE_RACE when optimistic lock loses', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index b13c1fe0..69b56e4f 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -6001,8 +6001,17 @@ async function commitLinkTransactionJournalEntry( if (!outcome.ok) { const entry = getErrorEntry(outcome.code) const httpStatus = entry?.httpStatus ?? 500 + // Carry the DB reason into the message itself: the dispatcher persists and + // returns `error`/`errorCode`, and the MCP approve result has no separate + // details slot, so a bare "database error" left the agent with nothing to + // act on. + const reason = outcome.details && typeof outcome.details.reason === 'string' + ? outcome.details.reason + : null + const baseMessage = entry?.message_en ?? outcome.code return { - error: entry?.message_en ?? outcome.code, + error: reason ? `${baseMessage} (${reason})` : baseMessage, + errorCode: outcome.code, status: httpStatus, data: outcome.details as Record | undefined, } @@ -6428,26 +6437,52 @@ async function commitPendingOperationInner( } } const isAutoReject = result.status === 404 || result.status === 409 - await supabase + // Executor-provided failure details (e.g. the Postgres message behind + // LINK_TX_DB_ERROR) are persisted and returned alongside the message so + // the approver sees WHY, not just that it failed. + const failureDetails = + result.data && Object.keys(result.data).length > 0 ? result.data : null + const { error: rejectWriteError } = await supabase .from('pending_operations') .update({ status: 'rejected', resolved_at: new Date().toISOString(), result_data: isAutoReject - ? { auto_rejected: true, reason: result.error } + ? { + auto_rejected: true, + reason: result.error, + ...(result.errorCode ? { error_code: result.errorCode } : {}), + ...(failureDetails ? { details: failureDetails } : {}), + } : { error: result.error, http_status: result.status, ...(result.errorCode ? { error_code: result.errorCode } : {}), + ...(failureDetails ? { details: failureDetails } : {}), }, }) .eq('id', pendingOp.id) + if (rejectWriteError) { + // Same contract as the finalize branch below: the row stays in + // 'committing' and the daily recovery sweep + // (recover-stuck-committing.ts) resolves it; log loudly with the ids + // and the failure we could not persist so nothing is lost silently. + log.error('failed to mark pending_operation rejected (left in committing)', rejectWriteError, { + pendingOperationId: pendingOp.id, + operationType: pendingOp.operation_type, + companyId, + executorError: result.error, + executorErrorCode: result.errorCode ?? null, + }) + } if (isAutoReject) { return { status: 'rejected', auto_rejected: true, error: result.error, http_status: result.status, + ...(result.errorCode ? { code: result.errorCode } : {}), + ...(failureDetails ? { data: failureDetails } : {}), } } return { @@ -6455,6 +6490,7 @@ async function commitPendingOperationInner( error: result.error, http_status: result.status ?? 500, ...(result.errorCode ? { code: result.errorCode } : {}), + ...(failureDetails ? { data: failureDetails } : {}), } }