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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
1f6dd778e5
commit
9622382579
@@ -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 } : {}),
|
||||
}
|
||||
},
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -678,6 +678,16 @@ const LINK_TX_JE: Record<string, StructuredErrorEntry> = {
|
||||
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<string, StructuredErrorEntry> = {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, unknown> | 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 } : {}),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user