From 9c0117952e8d20779b2168e20e4200cba0cb9cb8 Mon Sep 17 00:00:00 2001 From: Matteo Date: Mon, 14 Sep 2026 14:33:31 +0200 Subject: [PATCH] feat(mcp): append an actionable hint to upstream errors the model keeps retrying Two failure shapes send AI clients into loops: weclapp's 'unknown property: X' (the model tries other spellings for ten calls; 194 failures in a week from one workspace) and Sorare's authenticate_from_new_country (no retry can fix it, only the account owner confirming the vendor's email). The tool result now carries a 'hint' next to the unmodified vendor body: how weclapp field names work and how to discover them, the expression vs query-string filter grammar, and the hand-over-to-the-user message for new-location refusals. Keyed on the upstream host so hand-built tools get the same help as catalog ones. Audit rows keep the raw cause only. --- .../src/mcp-server/dynamic-mcp-tools.spec.ts | 37 +++++ .../src/mcp-server/dynamic-mcp-tools.ts | 14 ++ .../src/mcp-server/error-hints.spec.ts | 73 ++++++++++ .../backend/src/mcp-server/error-hints.ts | 127 ++++++++++++++++++ 4 files changed, 251 insertions(+) create mode 100644 packages/backend/src/mcp-server/error-hints.spec.ts create mode 100644 packages/backend/src/mcp-server/error-hints.ts diff --git a/packages/backend/src/mcp-server/dynamic-mcp-tools.spec.ts b/packages/backend/src/mcp-server/dynamic-mcp-tools.spec.ts index d1c1d66d..74b44243 100644 --- a/packages/backend/src/mcp-server/dynamic-mcp-tools.spec.ts +++ b/packages/backend/src/mcp-server/dynamic-mcp-tools.spec.ts @@ -147,6 +147,43 @@ describe('DynamicMcpTools — response shaping', () => { }); }); +describe('DynamicMcpTools — actionable hint on upstream errors', () => { + it('appends a hint for a weclapp "unknown property" 400, next to the vendor body', async () => { + const { AxiosError } = await import('axios'); + const tool = makeTool(); + tool.connectorConfig = { baseUrl: 'https://purora.weclapp.com/webapp/api/v2', authType: 'NONE' }; + const { executor, restEngine, audit } = build(tool); + const err = new AxiosError('Request failed with status code 400', '400', { url: '/salesOrder', baseURL: tool.connectorConfig.baseUrl } as any, undefined, { + status: 400, + statusText: 'Bad Request', + headers: {}, + config: {} as any, + data: { detail: 'unknown property: orderItems.articleNumber', status: 400 }, + }); + restEngine.execute.mockRejectedValueOnce(err); + + const res = await executor.executeTool('list_devices', {}); + + expect(res.isError).toBe(true); + const detail = JSON.parse(res.content[0].text); + // The vendor's own message is still there, unmodified… + expect(detail.responseBody.detail).toBe('unknown property: orderItems.articleNumber'); + // …and the hint sits beside it. + expect(detail.hint).toMatch(/fetch ONE record/); + // The audit row keeps the raw upstream cause, not the hint. + expect(audit.logInvocation.mock.calls[0][0].error).toMatch(/unknown property/); + expect(audit.logInvocation.mock.calls[0][0].error).not.toMatch(/fetch ONE record/); + }); + + it('adds no hint for an ordinary failure on an unknown host', async () => { + const { executor, restEngine } = build(makeTool()); + restEngine.execute.mockRejectedValueOnce(new Error('boom')); + const res = await executor.executeTool('list_devices', {}); + expect(res.isError).toBe(true); + expect(JSON.parse(res.content[0].text).hint).toBeUndefined(); + }); +}); + describe('DynamicMcpTools — response cache', () => { it('caches the raw response, not the rendered text', async () => { const { executor, redis } = build(makeTool({ ...SELECT_TRANSFORM, cacheTtl: 300 })); diff --git a/packages/backend/src/mcp-server/dynamic-mcp-tools.ts b/packages/backend/src/mcp-server/dynamic-mcp-tools.ts index 5c3719e0..23066752 100644 --- a/packages/backend/src/mcp-server/dynamic-mcp-tools.ts +++ b/packages/backend/src/mcp-server/dynamic-mcp-tools.ts @@ -26,6 +26,7 @@ import { import { KgService } from '../knowledge-graph/kg.service'; import type { ResponseMapping } from '../connectors/engines/engine-types'; import type { RegisteredTool } from './tool-registry'; +import { deriveErrorHint, hostFromAxiosConfig, hostFromUrl } from './error-hints'; /** * ToolExecutor — executes dynamically registered MCP tools. @@ -353,6 +354,19 @@ export class DynamicMcpTools { const durationMs = Date.now() - startTime; const errorDetail = this.extractErrorDetail(error); + // One actionable line for the client, on top of the vendor's own body. + // Keyed on the upstream host so hand-built tools get the same help as + // catalog ones; see error-hints.ts for why this exists. + const hint = deriveErrorHint({ + host: + hostFromAxiosConfig(error?.config) ?? + hostFromUrl(tool.connectorConfig?.baseUrl), + status: typeof errorDetail.status === 'number' ? errorDetail.status : undefined, + message: typeof error?.message === 'string' ? error.message : undefined, + body: errorDetail.responseBody, + }); + if (hint) errorDetail.hint = hint; + await this.auditService.logInvocation({ toolId: tool.id, userId: context?.userId, diff --git a/packages/backend/src/mcp-server/error-hints.spec.ts b/packages/backend/src/mcp-server/error-hints.spec.ts new file mode 100644 index 00000000..b6b5309c --- /dev/null +++ b/packages/backend/src/mcp-server/error-hints.spec.ts @@ -0,0 +1,73 @@ +import { deriveErrorHint, hostFromAxiosConfig } from './error-hints'; + +describe('deriveErrorHint', () => { + it('says nothing for an ordinary failure', () => { + expect( + deriveErrorHint({ host: 'api.example.com', status: 500, body: { message: 'boom' } }), + ).toBeUndefined(); + }); + + describe('weclapp', () => { + it('explains unknown properties instead of letting the model guess spellings', () => { + const hint = deriveErrorHint({ + host: 'purora.weclapp.com', + status: 400, + body: { detail: 'unknown property: orderItems.articleNumber', status: 400 }, + }); + expect(hint).toMatch(/fetch ONE record/); + expect(hint).toMatch(/orderItems/); + }); + + it('treats "unexpected filter property" the same way', () => { + expect( + deriveErrorHint({ host: 'x.weclapp.com', body: 'unexpected filter property' }), + ).toMatch(/fetch ONE record/); + }); + + it('teaches the expression grammar when weclapp cannot parse the filter', () => { + const hint = deriveErrorHint({ + host: 'purora.weclapp.com', + status: 400, + body: { detail: 'Expression contains errors' }, + }); + expect(hint).toMatch(/~ "%pattern%"/); + expect(hint).toMatch(/like\/ilike do not exist/); + }); + + it('applies to hand-built tools on the same host, not only catalog ones', () => { + expect( + deriveErrorHint({ host: 'tenant.weclapp.com', body: 'unknown property: x' }), + ).toBeDefined(); + }); + + it('does not fire for another vendor that happens to say "unknown property"', () => { + expect( + deriveErrorHint({ host: 'api.other.com', body: 'unknown property: x' }), + ).toBeUndefined(); + }); + }); + + describe('login refused from a new country', () => { + it('tells the model to hand over to the user, regardless of host', () => { + const hint = deriveErrorHint({ + message: 'LOGIN_TOKEN: the service refused the login — authenticate_from_new_country', + }); + expect(hint).toMatch(/confirm the login/); + expect(hint).toMatch(/cannot be fixed by retrying/); + }); + }); +}); + +describe('hostFromAxiosConfig', () => { + it('reads an absolute url', () => { + expect(hostFromAxiosConfig({ url: 'https://a.weclapp.com/webapp/api/v2/article' })).toBe('a.weclapp.com'); + }); + + it('combines a relative url with the baseURL', () => { + expect(hostFromAxiosConfig({ baseURL: 'https://b.weclapp.com/webapp/api/v2', url: '/article' })).toBe('b.weclapp.com'); + }); + + it('is undefined without a config', () => { + expect(hostFromAxiosConfig(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/backend/src/mcp-server/error-hints.ts b/packages/backend/src/mcp-server/error-hints.ts new file mode 100644 index 00000000..6f6dc4c4 --- /dev/null +++ b/packages/backend/src/mcp-server/error-hints.ts @@ -0,0 +1,127 @@ +/** + * Turns a raw upstream failure into a one-line hint the AI client can act on. + * + * WHY. The tool result already carries the vendor's response body, and that is + * the right thing to keep: it names the real cause. But some vendors phrase it + * in a way that sends the model into a loop. weclapp answers "unknown property: + * articleNumber" and the model tries "articleName", "article.number", … for + * ten calls in a row (194 failures in one week from a single starter + * workspace). Sorare answers `authenticate_from_new_country`, which no model + * can fix by retrying — only the human can, by confirming the login from the + * vendor's email. The hint says what to do instead of what went wrong. + * + * WHERE IT APPLIES. Rules are keyed on the upstream host (so a hand-built tool + * against the same API gets the same help as a catalog one) or on the error + * text itself when the host is not known. Everything here is advisory: a hint + * is appended, never substituted for the vendor's own message. + */ + +export interface ErrorHintInput { + /** Upstream host the request went to, if known (e.g. `purora.weclapp.com`). */ + host?: string | null; + /** HTTP status of the upstream answer, if any. */ + status?: number; + /** The engine's own error message. */ + message?: string; + /** The upstream response body, raw. */ + body?: unknown; +} + +const WECLAPP_FIELD_HINT = + 'weclapp rejects fields it does not know. Field names are camelCase and belong to ' + + 'the entity itself: line items live under `orderItems` / `salesInvoiceItems` / ' + + '`shipmentItems` (as nested objects, not as filterable properties), the customer ' + + 'name is inside `recordAddress`, links end in `Id` (`customerId`, `articleId`, ' + + '`warehouseId` is NOT a field of every entity). Do not guess another spelling: ' + + 'fetch ONE record without `properties` and without that filter, read the field ' + + 'names it actually returns, then retry using only those.'; + +const WECLAPP_EXPRESSION_HINT = + 'This tool sends `filter` as a weclapp filter EXPRESSION, not as a query string. ' + + 'Grammar: property = "value" | property != "value" | property ~ "%pattern%" ' + + '(pattern match; the words like/ilike do not exist) | property in ["A","B"] | ' + + 'property > 5. Strings in double quotes. Do NOT write property-eq=value here. ' + + 'Example: name ~ "%Protein%".'; + +const WECLAPP_RAW_FILTER_HINT = + 'This endpoint takes weclapp filters as query parameters, one per condition: ' + + '`property-operator=value` (operators -eq -ne -gt -lt -ge -le -like -ilike -in ' + + '-notin -null -notnull), joined with `&`. `-in` needs a bracketed list: ' + + '`id-in=[1,2]`. Do not send an SQL-like expression.'; + +const NEW_COUNTRY_HINT = + 'The service refused the sign-in because it came from a new location (the ' + + 'connector signs in from the AnythingMCP server, not from the user\'s device). ' + + 'This cannot be fixed by retrying or by changing the request. The account owner ' + + 'must open the email the service just sent ("new country / new device") and ' + + 'confirm the login, then the tool works. Tell the user exactly that.'; + +function bodyText(body: unknown): string { + if (body === undefined || body === null) return ''; + if (typeof body === 'string') return body; + try { + return JSON.stringify(body); + } catch { + return String(body); + } +} + +function hostMatches(host: string | null | undefined, suffix: string): boolean { + if (!host) return false; + const h = host.toLowerCase(); + return h === suffix || h.endsWith(`.${suffix}`); +} + +/** + * The hint for a failed upstream call, or `undefined` when nothing useful can + * be said. Pure: safe to call from a catch block. + */ +export function deriveErrorHint(input: ErrorHintInput): string | undefined { + const text = `${input.message ?? ''}\n${bodyText(input.body)}`; + + // Login-token flows: the vendor's refusal reason is folded into our message. + if (/authenticate_from_new_country|new_country|unrecognized_device|new_device/i.test(text)) { + return NEW_COUNTRY_HINT; + } + + if (hostMatches(input.host, 'weclapp.com')) { + if (/unknown property|unexpected filter property/i.test(text)) { + return WECLAPP_FIELD_HINT; + } + if (/expression contains errors/i.test(text)) { + return WECLAPP_EXPRESSION_HINT; + } + if (/invalid parameter value|unknown query parameter|unexpected parameter/i.test(text)) { + return WECLAPP_RAW_FILTER_HINT; + } + } + + return undefined; +} + +/** Hostname of a URL, or undefined when it is not one (e.g. still holds `{{VAR}}`). */ +export function hostFromUrl(url: string | null | undefined): string | undefined { + if (!url) return undefined; + try { + return new URL(url).hostname; + } catch { + return undefined; + } +} + +/** + * Best-effort host extraction from an axios error: the request config carries + * either an absolute `url` or a `baseURL` + relative `url`. + */ +export function hostFromAxiosConfig(config: { url?: string; baseURL?: string } | undefined): string | undefined { + if (!config) return undefined; + for (const candidate of [config.url, config.baseURL]) { + if (!candidate) continue; + try { + return new URL(candidate, config.baseURL || undefined).hostname; + } catch { + /* relative url without a base: try the next candidate */ + } + } + return undefined; +}