fix(mcp): give the missing-chart-accounts refusal a code agents can dispatch on (#2075)
getStructuredError resolves an error with no `code` to UNKNOWN_ERROR, whose registry text is the constant "Något gick fel. Försök igen." So an agent branching on `code` saw "unknown" for a failure whose own message already named the accounts and the two tools that fix it. On production that was 40 of the create_voucher failures in 60 days. Everything needed already existed: the ACCOUNTS_NOT_IN_CHART code, its registry entry, its remediation pointing at the chart-of-accounts resource, and a typed error class that storno-service already throws. This path just never attached the code. Attached with Object.assign rather than by throwing AccountsNotInChartError, because that class builds its own generic English message from the account list and would discard the richer Swedish one, which is the more useful half. Pinned by a test asserting the same message WITHOUT the code still resolves to UNKNOWN_ERROR, so this cannot silently regress. Not fixed here: the MCP server has ~155 bare Swedish-prose throws against 3 uses of codedError(), so most domain failures remain undispatchable. This fixes the one instance telemetry actually proved rather than converting 155 sites blind. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Opus 5
parent
413a0978c0
commit
523beaa18b
@@ -1391,6 +1391,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-08-31] Login/register methods come from GoTrue (/auth/v1/settings + admin customProviders) instead of app-side flags; NEXT_PUBLIC_GOOGLE_AUTH_ENABLED removed (PR #1869): the Supabase dashboard becomes the single switch, an allowlist of auth-js provider ids filters non-login entries like anonymous_users, and hosted rendering is unchanged because Google is enabled in prod GoTrue. The Vercel env var stays set for old-build rollback safety; delete it after a few deploys.
|
||||
[2026-08-31] Single prominent amount is PROMOTED into editable totals.total (totalSource='prominent') instead of living in a read-only Belopp row: Emil's call, an uncorrectable load-bearing value violated the prefill-override-editors rule. Provenance keeps matching fallback-grade (discount, date guard, hunt exclusion); a user edit of TOTALT clears the stamp. Multi-amount docs keep the Belopp row: promoting one of several figures would invent a total.
|
||||
[2026-08-31] Image-scan red fixed by bumping the node:22-alpine digest (alpine 3.23 to 3.24.1), not by widening the gate: the Dockerfile's apk-upgrade layer is frozen by the GHCR buildx layer cache, so a fix published after the last cache-busting change (libssl3 3.5.8-r0 for CVE-2026-14456) never reaches the published image until the FROM digest moves; the red scheduled scan is the designed alarm for exactly this bump. cron.Dockerfile gained the same apk upgrade (it had none).
|
||||
[2026-08-31] gnubok_create_voucher's "Konton saknas i kontoplanen" refusal now carries the ACCOUNTS_NOT_IN_CHART code instead of being a bare Error. getStructuredError resolves an uncoded error to UNKNOWN_ERROR, whose registry text is the constant "Något gick fel. Försök igen.", so an agent branching on `code` saw "unknown" for a failure whose own message already named the accounts AND the two tools that fix it (gnubok_create_account / gnubok_update_account). On prod that was 40 of the create_voucher failures in 60 days. The code, its registry entry and its remediation (pointing at Accounted://chart-of-accounts) all already existed, and storno-service already threw AccountsNotInChartError; this path simply never attached it. Attached via Object.assign rather than throwing AccountsNotInChartError, because that class builds its own generic English message from the account list and would have discarded the richer Swedish one, which is the more useful half. Pinned by a test asserting the same message WITHOUT the code still resolves to UNKNOWN_ERROR, so the fix cannot silently regress. Broader pattern recorded and NOT fixed here: the MCP server has ~155 bare `throw new Error('<Swedish prose>')` against 3 uses of codedError(), so most domain failures are still undispatchable; this change fixes the one instance telemetry actually proved rather than converting 155 sites blind. NOTE the originally planned work here was MCP input_examples (Anthropic's 72%->90% finding), DROPPED after checking our own telemetry: VALIDATION_ERROR totals roughly 50 events across 60 days over all tools, so parameter shape is demonstrably not where agents struggle; uncoded domain errors are.
|
||||
[2026-08-31] Every v1 REST handler now runs inside a runWithActor({type:'api_key', label:<key name>}) scope, set once in withApiV1 rather than threaded through each route. commitEntry() already reads getActor() as its fallback and forwards it to commit_journal_entry, which stamps journal_entries.committed_actor_* and the audit_log COMMIT row (20260619120000), so one wrapper attributes EVERY v1 write, including the ones that reach the ledger several frames down via reverseEntry/correctEntry/the supplier-invoice paths. No signature changes, no migration. Before this, runWithActor had exactly ONE production call site (lib/pending-operations/commit.ts:6358), so everything committing outside the staged path was anonymous. Measured on prod over 90 days: 99.8% of storno entries and 100% of correction entries had no actor, which are precisely the two sanctioned rättelse paths under BFL 5 kap. 5 § and the place where "who did this, and when" is a legal question rather than a nicety. NOTE the headline "98.8% of all entries" from the original issue was true but misleading: 184 873 of the 193 466 unattributed entries are source_type='import' (bulk SIE migration, no meaningful actor); excluding import it is 8 593 of 11 009 non-import entries, and the severity lives in storno/correction, not in the aggregate. Wrapped at the authenticated invocation only, not the anonymous/public branch, where no attribution is meaningful. actor-context-node's own docstring names API routes as a sanctioned import site (it is server-only; the isomorphic half exists so engine.ts stays client-bundle safe). Verified the test discriminates by removing the wrapper and watching 3 of 4 fail, while the AsyncLocalStorage leak check correctly still passed.
|
||||
[2026-08-31] Agent approval-authority ceiling stored on api_keys, not company_settings: a company-scoped money threshold catches humans too, and company_settings.agent_auto_commit_max_amount was already tried (20260501120000) and dropped four days later (20260505190027) for exactly that reason.
|
||||
[2026-08-31] Ceiling enforced in TypeScript at the two commit doors, not inside commit_journal_entry: a RAISE there is swallowed by engine.ts into BookkeepingDatabaseError (500, retryable), and on the MCP path that burns the pending op to terminal 'rejected', destroying the staged verifikat. Both doors refuse BEFORE the atomic claim / before commitEntry, so the work survives as 'pending' or as a draft and a human can approve the same verifikat.
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
/**
|
||||
* Domain failures must carry a stable code, not just Swedish prose.
|
||||
*
|
||||
* `getStructuredError` resolves an error with no `code` to UNKNOWN_ERROR, whose
|
||||
* registry message is the constant "Något gick fel. Försök igen." An agent
|
||||
* branching on `code` therefore sees "unknown" even when the message it was
|
||||
* handed says exactly what to do.
|
||||
*
|
||||
* Production, 60 days, bot actors excluded: UNKNOWN_ERROR was 645 of 1 024
|
||||
* real-agent failures. 40 of those were gnubok_create_voucher refusing a
|
||||
* booking because the BAS accounts were not active in the company's chart, a
|
||||
* failure that already had a registry code (ACCOUNTS_NOT_IN_CHART) with a
|
||||
* remediation, and a message naming the two tools that fix it. The code was
|
||||
* simply never attached.
|
||||
*
|
||||
* The MCP server currently has ~155 bare `throw new Error('<Swedish prose>')`
|
||||
* against 3 uses of codedError(), so this is one instance of a broad pattern.
|
||||
* This file pins the instance that telemetry proved, and the shape any future
|
||||
* fix should follow.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { getStructuredError } from '@/lib/errors/get-structured-error'
|
||||
import { ACCOUNTS_NOT_IN_CHART } from '@/lib/bookkeeping/errors'
|
||||
|
||||
/** The exact shape extensions/general/mcp-server/server.ts now throws. */
|
||||
function missingAccountsError(accounts: string[]) {
|
||||
return Object.assign(
|
||||
new Error(
|
||||
`Kan inte skapa verifikation. Konton saknas i kontoplanen och finns inte i BAS 2026: ${accounts.join(', ')}. ` +
|
||||
'Skapa kontot med gnubok_create_account, aktivera det med gnubok_update_account, eller välj andra konton.',
|
||||
),
|
||||
{ code: ACCOUNTS_NOT_IN_CHART, accountNumbers: accounts },
|
||||
)
|
||||
}
|
||||
|
||||
describe('missing chart accounts is a dispatchable failure', () => {
|
||||
it('resolves to ACCOUNTS_NOT_IN_CHART, not UNKNOWN_ERROR', () => {
|
||||
expect(getStructuredError(missingAccountsError(['4010'])).code).toBe('ACCOUNTS_NOT_IN_CHART')
|
||||
})
|
||||
|
||||
it('would have been UNKNOWN_ERROR without the code', () => {
|
||||
// The regression this pins. Same message, no code: an agent gets prose it
|
||||
// cannot branch on, which is what production showed for 40 calls.
|
||||
const uncoded = new Error(
|
||||
'Kan inte skapa verifikation. Konton saknas i kontoplanen och finns inte i BAS 2026: 4010.',
|
||||
)
|
||||
expect(getStructuredError(uncoded).code).toBe('UNKNOWN_ERROR')
|
||||
})
|
||||
|
||||
it('carries the registry remediation the agent can act on', () => {
|
||||
const structured = getStructuredError(missingAccountsError(['3010']))
|
||||
expect(structured.remediation?.description ?? '').toMatch(/activate/i)
|
||||
})
|
||||
|
||||
it('keeps the richer message that names the remedy tools', () => {
|
||||
// The registry's own text is the generic "Konton saknas i kontoplanen.".
|
||||
// The thrown message is better because it names both the accounts and the
|
||||
// tools, so it must survive being coded.
|
||||
const structured = getStructuredError(missingAccountsError(['7971']))
|
||||
const text = `${structured.message_sv} ${structured.message_en}`
|
||||
expect(text).toContain('gnubok_create_account')
|
||||
expect(text).toContain('7971')
|
||||
})
|
||||
})
|
||||
@@ -47,6 +47,7 @@ import { applyAccountOverride } from '@/lib/bookkeeping/account-override'
|
||||
import { ACCOUNT_NUMBER_RE } from '@/lib/invariants/account-number'
|
||||
import { isSlpPensionAccount } from '@/lib/bookkeeping/slp-lines'
|
||||
import { getErrorEntry } from '@/lib/errors/structured-errors'
|
||||
import { ACCOUNTS_NOT_IN_CHART } from '@/lib/bookkeeping/errors'
|
||||
import { dbError } from '@/lib/errors/db-error'
|
||||
import { getStructuredError } from '@/lib/errors/get-structured-error'
|
||||
import { applySettlementAccount } from '@/lib/bookkeeping/mapping-engine'
|
||||
@@ -18294,9 +18295,23 @@ export const tools: McpTool[] = [
|
||||
if (inactiveAccounts.length > 0) {
|
||||
parts.push(`inaktiva: ${inactiveAccounts.join(', ')}`)
|
||||
}
|
||||
throw new Error(
|
||||
`Kan inte skapa verifikation. Konton ${parts.join('; ')}. ` +
|
||||
'Skapa kontot med gnubok_create_account, aktivera det med gnubok_update_account, eller välj andra konton.'
|
||||
// Carry the stable code, not just the prose. The message here is the
|
||||
// better one (it names the accounts AND the two tools that fix it),
|
||||
// but a bare Error resolves to UNKNOWN_ERROR, so an agent branching on
|
||||
// `code` sees "unknown" for a failure that has a documented remedy.
|
||||
// ACCOUNTS_NOT_IN_CHART already exists in the registry with a
|
||||
// remediation pointing at Accounted://chart-of-accounts, and
|
||||
// storno-service already throws it; this path just never did.
|
||||
// On production this was 40 of the create_voucher failures in 60 days.
|
||||
throw Object.assign(
|
||||
new Error(
|
||||
`Kan inte skapa verifikation. Konton ${parts.join('; ')}. ` +
|
||||
'Skapa kontot med gnubok_create_account, aktivera det med gnubok_update_account, eller välj andra konton.'
|
||||
),
|
||||
{
|
||||
code: ACCOUNTS_NOT_IN_CHART,
|
||||
accountNumbers: [...unseedableAccounts, ...inactiveAccounts],
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user