fix(mcp): tag unmapped tool failures with their cause vocabulary (#2051) (#2135)

Closes #2051. errorCauseTag() shipped in #2027 written and tested but
wired to nothing. This connects it: the two execution catch paths (sync
call and task) now pass errorCause into mcp.tool_called, carrying the
SQLSTATE or coded-error code, else the error's class name, capped at 64
chars.

The rows this exists for are the UNKNOWN_ERROR residue, whose
errorMessage is the constant "Något gick fel. Försök igen." and whose
errorDetail is the English constant: 465 such rows in the last 30 days
(create_voucher 58 of its 60 failures, query_journal 122) with nothing
to cluster on. A five-character SQLSTATE is protocol vocabulary; a raw
driver message can quote row values from a constraint violation and
belongs in the server log, never in event_log, so the raw message is
deliberately not captured.

A plain `new Error(...)` tags null rather than 'Error': tagging
everything is the same as tagging nothing. Pre-execution denials (scope,
capability, validation, unknown tool) pass nothing because their
errorCode already is the cause.

Self-tested by unwiring one call site and watching the new test name it.


Claude-Session: https://claude.ai/code/session_01L3P2hr19PhQuCoTSGoegcY

Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-09-01 22:07:46 +02:00
committed by GitHub
co-authored by Jakob Wennberg Claude Opus 5
parent 191fb2cfce
commit dd84d6c1bb
4 changed files with 71 additions and 2 deletions
@@ -388,6 +388,59 @@ describe('mcp.tool_called telemetry', () => {
expect(event.latencyMs).toBeGreaterThanOrEqual(0)
})
describe('errorCause: machine vocabulary for the unmapped residue (#2051)', () => {
/**
* 65.1% of real-agent errors in the 30 days before #2027 were
* UNKNOWN_ERROR, whose errorMessage is the constant "Något gick fel.
* Försök igen." and whose errorDetail is the English constant: nothing to
* cluster on. errorCause carries errorCauseTag(err): the SQLSTATE or coded
* error code, else the error's class name. Protocol vocabulary only,
* because a raw driver message can quote row values from a constraint
* violation and belongs in the server log, never in event_log.
*/
it('records the error class name when execute() dies unmapped', async () => {
const eventPromise = captureNextToolCalledEvent()
// Valid call for the key's reports:read scope; the harness supabase is a
// bare vi.fn() mock, so execute() dies on it. Exactly the shape that
// used to log UNKNOWN_ERROR with the constant message and nothing else.
await handleMcpRequest(
mcpRequest('tools/call', { name: 'gnubok_get_trial_balance', arguments: {} })
)
const event = await eventPromise
expect(event.errorKind).toBe('execution')
expect(event.errorCause).toBe('TypeError')
})
it('stays null for a plain Error: the class name "Error" is noise, not vocabulary', async () => {
const eventPromise = captureNextToolCalledEvent()
// gnubok_load_skill throws `new Error("Skill not found: ...")`.
await handleMcpRequest(
mcpRequest('tools/call', { name: 'gnubok_load_skill', arguments: { slug: 'definitely-does-not-exist' } })
)
const event = await eventPromise
expect(event.errorKind).toBe('execution')
expect(event.errorCause).toBeNull()
})
it('stays null on success and on pre-execution denials', async () => {
const successPromise = captureNextToolCalledEvent()
await handleMcpRequest(mcpRequest('tools/call', { name: 'gnubok_list_skills', arguments: {} }))
expect((await successPromise).errorCause).toBeNull()
const deniedPromise = captureNextToolCalledEvent()
await handleMcpRequest(
mcpRequest('tools/call', { name: 'gnubok_create_invoice', arguments: { customer_id: 'x', items: [] } })
)
const denied = await deniedPromise
expect(denied.errorKind).toBe('scope_denied')
expect(denied.errorCause).toBeNull()
})
})
it('does NOT block the JSON-RPC response on telemetry: even if a handler throws', async () => {
// Register a handler that throws synchronously. The bus already isolates
// failures via Promise.allSettled, so the response should still arrive.