fix(mcp): supplier-invoice-from-inbox resolves FX through the shared resolver, with the cache and an override (#2173)
* fix(mcp): supplier-invoice-from-inbox resolves FX through the shared resolver, with the cache and an override MCP feedback seq 299742: eight USD supplier invoices staged from the inbox in one batch, three got a Riksbanken rate and five came back exchange_rate: null / exchange_rate_source "lookup_failed", reproducibly, for ordinary weekdays in April-August. None of the five could be approved (the executor refuses with SI_FX_RATE_MISSING; it never books 0 SEK), and the tool offered no way to supply the rate. Cause: the tool called fetchExchangeRate without the supabase client, so neither the shared exchange_rates read-through cache nor the last-cached-observation fallback was reachable. Riksbanken's IP limiter answers 429 after about five requests in a burst (a weekend date costs two: exact-date 204, then the 7-day range), sends no Retry-After header, and asks for ~54 s, which the 5 s retry cap cannot honour. The pass/fail split was request ordering, nothing about the dates. - Resolve through resolveSupplierInvoiceExchangeRate with the client, the same resolver the commit executor and the v1/web write paths use, so the staging preview and the commit agree and the cache is consulted and warmed. - New input exchange_rate_override (SEK per 1 unit of invoice currency), trusted verbatim like the web form and v1; validated positive and finite, refused as implausible past the resolver's bound, rejected on a SEK invoice. Source is echoed as "supplied". - When the lookup still fails, the preview carries exchange_rate_hint saying approval will refuse and naming the override that unblocks it. tools/list: +1 property (~25 tokens), ledger line added in payload-size.bench.test.ts; ceiling unchanged. The retry cap is left as is: waiting a minute inside a tool call or the sync cron's fan-out is a design call, and the cached fallback now covers the common case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yw62FMXGSzo6icFDiBwP3 * docs(decisions): FX override and retry cap on the inbox supplier-invoice tool Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013yw62FMXGSzo6icFDiBwP3 --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Fable 5.1
Jakob Wennberg
Claude Fable 5.1
Jakob Wennberg
parent
227a6317f1
commit
7240bfe7f3
@@ -1513,6 +1513,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-09-02] Agent-triggerable bank sync shipped (v1 POST /bank-connections/{id}/sync + MCP gnubok_sync_bank), lifting the 2026-09-01 deferral: Emil chose to close every open F2 item in one PR. The cost worry is bounded structurally instead of by policy: the window is never caller-controlled (gap-aware 7 to 90 days, same helper as the cron), a connection synced within 15 minutes answers BANK_SYNC_COOLDOWN with next_allowed_at (MCP returns it in-band as synced=false so agents read on instead of retrying), and failures are throttled per process by attempt time. The web Synka-nu route is left untouched rather than refactored onto the shared runner: it carries UI-only behaviour (caller-chosen days_back up to 365, SIE sweep stamping) and a regression there would hit every user for a code-sharing win.
|
||||
[2026-09-02] Bank-sync cooldown is a durable lease column (bank_connections.sync_lease_until, migration 20260902150000) claimed with one conditional UPDATE, not a process-local attempt map: the security scan on PR #2165 showed the map is bypassed by a second serverless instance or a cold start, so two agent calls could each bill Enable Banking. A column add was chosen over reusing extension_data because PostgREST cannot express an atomic conditional upsert there; the nightly cron deliberately ignores the lease.
|
||||
[2026-09-02] Grok links carry auth=required like the claude.ai link (#2159), decided from a live test: on the lazy URL Grok's connector dialog listed all 150+ tools and never opened the sign-in, so it reads the 200 probe as an authless server exactly as claude.ai does. The flag lives in one helper (mcpServerUrl / sideDoorServerUrl in lib/onboarding/checklist.ts) so the settings row, the onboarding side door and the deep link cannot drift; ChatGPT stays lazy because its developer mode honours the 401 on the first protected call.
|
||||
[2026-09-02] create_supplier_invoice_from_inbox gained exchange_rate_override (+~25 tokens against a tools/list budget at zero headroom) and the Riksbanken retry cap stayed at 5 s: a staged foreign invoice with no rate is unapprovable (SI_FX_RATE_MISSING) and the override is the only agent-side unblock, while honouring the limiter's 54 s ask would park a tool call or the sync cron's fan-out for a minute per 429; passing the supabase client to the shared resolver (cache + last-cached fallback) removes the common case instead.
|
||||
[2026-09-02] reconcile_match staging folds N:1 groups back together by journal_entry_id (links from the dry run that share a verifikat and carry no allocated_amount), mirroring the existing bank 1:N fold, instead of threading the caller's original pairs through: the dry run is the source of truth for what was validated, a verifikat can only be settled once so shared-JE links can only have come from one N:1 pair, and use_proposals has no caller pairs to thread. The remaining gap (dry run validates only pair shape for non-split pairs) is the engine's to close, not the tool's.
|
||||
[2026-09-02] parties children/roles reference parties(id, company_id) with composite FKs, not parties(id): a party UUID from another tenant is rejected by construction instead of relying on each writer to check; ON DELETE SET NULL (party_id) on customers/suppliers because a plain SET NULL would null company_id too (Superagent P2 on #2162)
|
||||
[2026-09-02] Party suggestions attach only by explicit party_id, org number or an exact ledger key already in alias_keys; same-core text is reported as similar_to for a person to decide and identities are withheld when a key mixes org numbers: the selection eval measured 9% false merges on trade names shared by distinct legal entities (Fortnox AB / Fortnox Finans), so text never merges
|
||||
|
||||
@@ -14,6 +14,12 @@ vi.mock('@/lib/currency/riksbanken', () => ({
|
||||
convertToSEK: vi.fn(),
|
||||
}))
|
||||
|
||||
const mockResolveRate = vi.fn()
|
||||
vi.mock('@/lib/currency/supplier-invoice-rate', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('@/lib/currency/supplier-invoice-rate')>()
|
||||
return { ...actual, resolveSupplierInvoiceExchangeRate: (...args: unknown[]) => mockResolveRate(...args) }
|
||||
})
|
||||
|
||||
describe('gnubok_create_supplier_invoice_from_inbox: registration', () => {
|
||||
it('is registered with idempotent + non-read-only annotations', () => {
|
||||
const tool = tools.find((t) => t.name === 'gnubok_create_supplier_invoice_from_inbox')
|
||||
@@ -779,3 +785,130 @@ describe('gnubok_create_supplier_invoice_from_inbox: execute', () => {
|
||||
).rejects.toThrow(/no extracted_data/)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* FX resolution at staging time (feedback seq 299742): the tool used to call
|
||||
* fetchExchangeRate WITHOUT the supabase client, so neither the shared
|
||||
* exchange_rates cache nor the last-cached-observation fallback was reachable
|
||||
* and a Riksbanken 429 surfaced as exchange_rate: null / lookup_failed for a
|
||||
* date that resolved fine a minute later. The staged op was then unapprovable
|
||||
* (SI_FX_RATE_MISSING at commit) with no way to supply the rate.
|
||||
*/
|
||||
describe('gnubok_create_supplier_invoice_from_inbox: exchange rate resolution', () => {
|
||||
const usdExtracted = {
|
||||
...baseExtracted,
|
||||
invoice: { ...baseExtracted.invoice, currency: 'USD', invoiceDate: '2026-04-24' },
|
||||
}
|
||||
const usdInbox = {
|
||||
id: 'inbox-usd',
|
||||
status: 'received',
|
||||
extracted_data: usdExtracted,
|
||||
matched_supplier_id: 'supplier-1',
|
||||
created_supplier_invoice_id: null,
|
||||
document_id: 'doc-usd',
|
||||
}
|
||||
const tool = () => tools.find((t) => t.name === 'gnubok_create_supplier_invoice_from_inbox')!
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockResolveRate.mockReset()
|
||||
})
|
||||
|
||||
it('resolves through the shared resolver WITH the supabase client (cache + last-cached fallback reachable)', async () => {
|
||||
mockResolveRate.mockResolvedValue({
|
||||
ok: true,
|
||||
rate: { currency: 'USD', rate: 9.51731, exchangeRate: 9.51731, exchangeRateDate: '2026-04-24', source: 'fetched' },
|
||||
})
|
||||
const supabase = makeMock({ inbox: usdInbox })
|
||||
const result = (await tool().execute(
|
||||
{ inbox_item_id: 'inbox-usd', dry_run: true },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase,
|
||||
)) as { preview: Record<string, unknown> }
|
||||
|
||||
expect(mockResolveRate).toHaveBeenCalledWith(supabase, {
|
||||
currency: 'USD',
|
||||
invoiceDate: '2026-04-24',
|
||||
suppliedRate: null,
|
||||
})
|
||||
expect(result.preview.exchange_rate).toBe(9.51731)
|
||||
expect(result.preview.exchange_rate_source).toBe('riksbanken')
|
||||
expect(result.preview.exchange_rate_hint).toBeUndefined()
|
||||
})
|
||||
|
||||
it('exchange_rate_override is passed as the supplied rate and echoed as source "supplied"', async () => {
|
||||
mockResolveRate.mockResolvedValue({
|
||||
ok: true,
|
||||
rate: { currency: 'USD', rate: 9.6, exchangeRate: 9.6, exchangeRateDate: null, source: 'supplied' },
|
||||
})
|
||||
const inserts: Array<Record<string, unknown>> = []
|
||||
const supabase = makeMock({ inbox: usdInbox, inserts })
|
||||
const result = (await tool().execute(
|
||||
{ inbox_item_id: 'inbox-usd', exchange_rate_override: 9.6 },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase,
|
||||
)) as { preview: Record<string, unknown> }
|
||||
|
||||
expect(mockResolveRate).toHaveBeenCalledWith(supabase, {
|
||||
currency: 'USD',
|
||||
invoiceDate: '2026-04-24',
|
||||
suppliedRate: 9.6,
|
||||
})
|
||||
expect(result.preview.exchange_rate).toBe(9.6)
|
||||
expect(result.preview.exchange_rate_source).toBe('supplied')
|
||||
// The staged params carry the rate the reviewer saw, so the executor's
|
||||
// resolver trusts it verbatim instead of re-fetching.
|
||||
const staged = inserts[0]?.params as Record<string, unknown> | undefined
|
||||
expect(staged?.exchange_rate).toBe(9.6)
|
||||
})
|
||||
|
||||
it('lookup failure stays visible as lookup_failed and tells the agent what unblocks approval', async () => {
|
||||
mockResolveRate.mockResolvedValue({ ok: false, currency: 'USD', invoiceDate: '2026-04-24' })
|
||||
const supabase = makeMock({ inbox: usdInbox })
|
||||
const result = (await tool().execute(
|
||||
{ inbox_item_id: 'inbox-usd', dry_run: true },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase,
|
||||
)) as { preview: Record<string, unknown> }
|
||||
|
||||
expect(result.preview.exchange_rate).toBeNull()
|
||||
expect(result.preview.exchange_rate_source).toBe('lookup_failed')
|
||||
expect(result.preview.exchange_rate_hint).toMatch(/exchange_rate_override \(SEK per 1 USD\)/)
|
||||
})
|
||||
|
||||
it('rejects a non-positive or non-numeric exchange_rate_override before touching the resolver', async () => {
|
||||
const supabase = makeMock({ inbox: usdInbox })
|
||||
await expect(
|
||||
tool().execute({ inbox_item_id: 'inbox-usd', exchange_rate_override: '9,6' }, 'company-1', 'user-1', supabase),
|
||||
).rejects.toThrow(/exchange_rate_override must be a positive number \(SEK per 1 USD\); got "9,6"/)
|
||||
await expect(
|
||||
tool().execute({ inbox_item_id: 'inbox-usd', exchange_rate_override: 0 }, 'company-1', 'user-1', supabase),
|
||||
).rejects.toThrow(/must be a positive number/)
|
||||
expect(mockResolveRate).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('an implausible override is refused with a pointer at the invoice, never silently replaced', async () => {
|
||||
mockResolveRate.mockResolvedValue({ ok: false, currency: 'USD', invoiceDate: '2026-04-24' })
|
||||
const supabase = makeMock({ inbox: usdInbox })
|
||||
await expect(
|
||||
tool().execute({ inbox_item_id: 'inbox-usd', exchange_rate_override: 250000 }, 'company-1', 'user-1', supabase),
|
||||
).rejects.toThrow(/exchange_rate_override 250000 was refused as implausible/)
|
||||
})
|
||||
|
||||
it('a SEK invoice does not consult the resolver and reports not_applicable', async () => {
|
||||
const supabase = makeMock({
|
||||
inbox: { ...usdInbox, id: 'inbox-sek', extracted_data: baseExtracted },
|
||||
})
|
||||
const result = (await tool().execute(
|
||||
{ inbox_item_id: 'inbox-sek', dry_run: true },
|
||||
'company-1',
|
||||
'user-1',
|
||||
supabase,
|
||||
)) as { preview: Record<string, unknown> }
|
||||
expect(mockResolveRate).not.toHaveBeenCalled()
|
||||
expect(result.preview.exchange_rate_source).toBe('not_applicable')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -382,6 +382,14 @@ describe('tools/list payload size guard', () => {
|
||||
// pair's examples ARE the two-step flow, and
|
||||
// list_uncategorized_transactions is the highest-traffic read.
|
||||
//
|
||||
// * +1 property, ~25 tokens (2026-09-02, MCP feedback seq 299742):
|
||||
// exchange_rate_override on create_supplier_invoice_from_inbox. A
|
||||
// foreign supplier invoice whose Riksbanken lookup fails stages with
|
||||
// exchange_rate: null and can never be approved (SI_FX_RATE_MISSING);
|
||||
// the property is the only way to unblock it from an agent. One short
|
||||
// description, no example; the lookup itself now goes through the
|
||||
// shared resolver with the cache, so the case is also rarer.
|
||||
//
|
||||
// Long-term answer to growth is no longer a ceiling bump. gnubok_call_tool
|
||||
// makes `catalogVisibility: 'search'` usable for READ tools on hosts that
|
||||
// can only invoke what tools/list showed them, which is the constraint that
|
||||
|
||||
@@ -63,7 +63,7 @@ import { eventBus } from '@/lib/events/bus'
|
||||
import { getVatRules, getPermittedVatRates, getArticleVatRateAdoptionSet } from '@/lib/invoices/vat-rules'
|
||||
import { validateDeductionLines } from '@/lib/invoices/rot-rut-rules'
|
||||
import { computeLineNet } from '@/lib/invoices/line-amounts'
|
||||
import { fetchExchangeRate } from '@/lib/currency/riksbanken'
|
||||
import { resolveSupplierInvoiceExchangeRate } from '@/lib/currency/supplier-invoice-rate'
|
||||
import { getBranding } from '@/lib/branding/service'
|
||||
import { generateIncomeStatement } from '@/lib/reports/income-statement'
|
||||
import {
|
||||
@@ -13190,6 +13190,7 @@ export const tools: McpTool[] = [
|
||||
vat_treatment_override: { type: 'string', enum: ['standard_25', 'reduced_12', 'reduced_6', 'reverse_charge', 'export', 'exempt'], description: 'Override extracted VAT treatment' },
|
||||
invoice_date_override: { type: 'string', description: 'Override extracted invoice date (YYYY-MM-DD). Use when OCR misses the date.' },
|
||||
due_date_override: { type: 'string', description: 'Override extracted due date (YYYY-MM-DD)' },
|
||||
exchange_rate_override: { type: 'number', description: 'SEK per 1 unit of invoice currency; skips the Riksbanken lookup.' },
|
||||
line_overrides: {
|
||||
type: 'array',
|
||||
description: 'Per-line overrides (1-based line_number): account_number wins over accountSuggestion and supplier default; dimensions tags that line; apply_slp books särskild löneskatt on a 741x pension line.',
|
||||
@@ -13386,14 +13387,40 @@ export const tools: McpTool[] = [
|
||||
?? (invoiceExt?.vatTreatment as string | undefined)
|
||||
?? 'standard_25'
|
||||
|
||||
// FX: if non-SEK, fetch rate at fakturadatum (best-effort; agent can re-stage on failure)
|
||||
// FX: a non-SEK invoice needs a rate before approve can post it (the
|
||||
// executor refuses with SI_FX_RATE_MISSING otherwise). Resolved through
|
||||
// the same resolver the commit path uses, WITH the supabase client: the
|
||||
// shared exchange_rates cache is consulted and warmed, and Riksbanken's
|
||||
// rate limiter (429, no Retry-After header) falls back to the most
|
||||
// recent cached observation instead of surfacing as lookup_failed. The
|
||||
// old call omitted the client, so five of eight USD invoices in one
|
||||
// batch staged with exchange_rate: null for no reason but request
|
||||
// ordering, and none of them could be approved (feedback seq 299742).
|
||||
// A caller-supplied exchange_rate_override is trusted verbatim, same as
|
||||
// the v1 route and the web form.
|
||||
let exchangeRate: number | null = null
|
||||
let exchangeRateSource: 'riksbanken' | 'supplied' | 'not_applicable' | 'lookup_failed' =
|
||||
currency === 'SEK' ? 'not_applicable' : 'lookup_failed'
|
||||
const rateOverride = args.exchange_rate_override
|
||||
if (rateOverride !== undefined && rateOverride !== null) {
|
||||
if (typeof rateOverride !== 'number' || !Number.isFinite(rateOverride) || rateOverride <= 0) {
|
||||
throw new Error(`exchange_rate_override must be a positive number (SEK per 1 ${currency}); got ${JSON.stringify(rateOverride)}`)
|
||||
}
|
||||
if (currency === 'SEK') {
|
||||
throw new Error('exchange_rate_override only applies to a non-SEK invoice')
|
||||
}
|
||||
}
|
||||
if (currency !== 'SEK' && invoiceDate) {
|
||||
try {
|
||||
const result = await fetchExchangeRate(currency as Currency, new Date(invoiceDate))
|
||||
exchangeRate = result?.rate ?? null
|
||||
} catch {
|
||||
exchangeRate = null // Agent will be informed via preview; can override later
|
||||
const fx = await resolveSupplierInvoiceExchangeRate(supabase, {
|
||||
currency,
|
||||
invoiceDate,
|
||||
suppliedRate: typeof rateOverride === 'number' ? rateOverride : null,
|
||||
})
|
||||
if (fx.ok && fx.rate.exchangeRate !== null) {
|
||||
exchangeRate = fx.rate.exchangeRate
|
||||
exchangeRateSource = fx.rate.source === 'supplied' ? 'supplied' : 'riksbanken'
|
||||
} else if (typeof rateOverride === 'number') {
|
||||
throw new Error(`exchange_rate_override ${rateOverride} was refused as implausible; check the rate on the invoice`)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13516,7 +13543,16 @@ export const tools: McpTool[] = [
|
||||
due_date: dueDate,
|
||||
currency,
|
||||
exchange_rate: exchangeRate,
|
||||
exchange_rate_source: exchangeRate !== null ? 'riksbanken' : currency === 'SEK' ? 'not_applicable' : 'lookup_failed',
|
||||
exchange_rate_source: exchangeRateSource,
|
||||
// Without a rate the staged op cannot be approved (SI_FX_RATE_MISSING),
|
||||
// so say what unblocks it here rather than at commit time.
|
||||
...(exchangeRateSource === 'lookup_failed'
|
||||
? {
|
||||
exchange_rate_hint:
|
||||
`No ${currency}/SEK rate for ${invoiceDate ?? 'the invoice date'} (Riksbanken unreachable or no observation). ` +
|
||||
'Approval will refuse; re-stage with exchange_rate_override (SEK per 1 ' + currency + ') taken from the invoice.',
|
||||
}
|
||||
: {}),
|
||||
vat_treatment: vatTreatment,
|
||||
subtotal: params.subtotal,
|
||||
vat_amount: params.vat_amount,
|
||||
|
||||
Reference in New Issue
Block a user