From bc7eb5a0ad7bdae1aedbd7ba22a34eb0a86c4072 Mon Sep 17 00:00:00 2001 From: Michal Date: Wed, 26 Aug 2026 00:11:50 +0100 Subject: [PATCH] fix(mcplocal): learn which models reject sampling params, don't hardcode them Auto-following to the newest Opus moved the failure rather than removing it. The deploy's own smoke output showed it: 404 "model not found" became HTTP 400: `temperature` is deprecated for this model. Anthropic removed temperature/top_p/top_k on the current generation (Opus 5, Sonnet 5, Opus 4.7/4.8, Fable 5), and the adapter sends temperature: 0 unconditionally. So the gate's prompt-selection was still degrading on every call -- just with a different status code. A list of which models accept sampling would rot exactly the way the pinned model ids did, which is the whole thing this branch is trying to stop. So the provider learns it instead: the first 400 naming a sampling parameter drops it and retries, and remembers the model so every later call omits it up front. One wasted call, once, rather than a hardcoded table to maintain. The match is deliberately narrow -- a 400 must actually name temperature/top_p/ top_k. An unrelated 400 (missing max_tokens, bad schema) propagates untouched; there is a test for that, because a broad match here would silently swallow real request errors and retry them pointlessly. Verified live: claude-opus-latest -> claude-opus-5, first attempt 400 on temperature, retry succeeds past it. The retry then hit HTTP 429 -- the personal OAuth token's rate limit, which is the pre-existing credential-tiering issue, not this path. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2 --- src/mcplocal/src/providers/anthropic.ts | 39 ++++++++++- .../tests/anthropic-model-resolution.test.ts | 65 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/mcplocal/src/providers/anthropic.ts b/src/mcplocal/src/providers/anthropic.ts index 7527051..73a60cc 100644 --- a/src/mcplocal/src/providers/anthropic.ts +++ b/src/mcplocal/src/providers/anthropic.ts @@ -30,6 +30,15 @@ function familyOf(model: string): string | null { return null; } +/** + * A 400 naming a sampling parameter, e.g. + * "`temperature` is deprecated for this model." + */ +function isSamplingRejection(err: unknown): boolean { + const msg = err instanceof Error ? err.message : String(err); + return msg.includes('HTTP 400') && /`?(temperature|top_p|top_k)`?/.test(msg); +} + /** Resolutions are cached this long before the models endpoint is consulted again. */ const MODEL_CACHE_TTL_MS = Number(process.env['MCPCTL_ANTHROPIC_MODEL_TTL_MS']) || 12 * 60 * 60 * 1000; @@ -40,6 +49,17 @@ export class AnthropicProvider implements LlmProvider { readonly name = 'anthropic'; /** Shared across instances: the model list is account-wide, not per-provider. */ private static readonly modelCache = new Map(); + /** + * Models that rejected a sampling parameter, learned at runtime. + * + * Anthropic removed `temperature`/`top_p`/`top_k` on the current generation + * (Opus 5, Sonnet 5, Opus 4.7/4.8, Fable 5) — sending one is a hard 400. A + * hardcoded list of which models accept it would rot exactly the way the + * pinned model ids did, so learn it from the API instead: the first call + * retries without the parameter and remembers, and every later call omits it + * up front. + */ + private static readonly rejectsSampling = new Set(); private apiKey: string; private defaultModel: string; @@ -66,7 +86,9 @@ export class AnthropicProvider implements LlmProvider { if (systemMessages.length > 0) { body.system = systemMessages.map((m) => m.content).join('\n'); } - if (options.temperature !== undefined) body.temperature = options.temperature; + if (options.temperature !== undefined && !AnthropicProvider.rejectsSampling.has(model)) { + body.temperature = options.temperature; + } if (options.tools && options.tools.length > 0) { body.tools = options.tools.map((t) => ({ @@ -76,8 +98,19 @@ export class AnthropicProvider implements LlmProvider { })); } - const response = await this.request(body, options.signal); - return parseAnthropicResponse(response); + try { + return parseAnthropicResponse(await this.request(body, options.signal)); + } catch (err) { + // Learn-and-retry rather than fail: a model that has dropped sampling + // support should cost one wasted call, once, not every call forever. + if (body.temperature !== undefined && isSamplingRejection(err)) { + AnthropicProvider.rejectsSampling.add(model); + delete body.temperature; + process.stderr.write(`[anthropic] ${model} rejects sampling params — retrying without\n`); + return parseAnthropicResponse(await this.request(body, options.signal)); + } + throw err; + } } /** diff --git a/src/mcplocal/tests/anthropic-model-resolution.test.ts b/src/mcplocal/tests/anthropic-model-resolution.test.ts index 1a6d7fa..89ea75a 100644 --- a/src/mcplocal/tests/anthropic-model-resolution.test.ts +++ b/src/mcplocal/tests/anthropic-model-resolution.test.ts @@ -81,3 +81,68 @@ describe('Anthropic model resolution', () => { await expect(providerWith(MODELS).listModels()).resolves.toContain('claude-opus-5'); }); }); + +describe('sampling-parameter rejection', () => { + afterEach(() => { + (AnthropicProvider as unknown as { rejectsSampling: Set }).rejectsSampling.clear(); + }); + + /** Rejects `temperature` exactly as the current Anthropic models do. */ + function samplingStrictProvider(): { provider: AnthropicProvider; bodies: Array> } { + const provider = new AnthropicProvider({ apiKey: 'sk-ant-api-test' }); + const bodies: Array> = []; + (provider as unknown as { request: (b: unknown) => Promise }).request = async (b) => { + const body = b as Record; + bodies.push({ ...body }); + if (body.temperature !== undefined) { + throw new Error( + 'Anthropic HTTP 400: {"type":"error","error":{"type":"invalid_request_error",' + + '"message":"`temperature` is deprecated for this model."}}', + ); + } + return { content: [{ type: 'text', text: 'ok' }], stop_reason: 'end_turn' }; + }; + return { provider, bodies }; + } + + it('retries without temperature when the model rejects it', async () => { + vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const { provider, bodies } = samplingStrictProvider(); + + const result = await provider.complete({ + model: 'claude-opus-5', + messages: [{ role: 'user', content: 'hi' }], + temperature: 0, + }); + + expect(result.content).toBe('ok'); + expect(bodies).toHaveLength(2); + expect(bodies[0]!.temperature).toBe(0); + expect(bodies[1]!.temperature).toBeUndefined(); + }); + + it('remembers, so the wasted call happens once and not forever', async () => { + vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const { provider, bodies } = samplingStrictProvider(); + const req = { model: 'claude-opus-5', messages: [{ role: 'user' as const, content: 'hi' }], temperature: 0 }; + + await provider.complete(req); + await provider.complete(req); + await provider.complete(req); + + // 2 for the first call (reject + retry), then 1 each — not 2 each. + expect(bodies).toHaveLength(4); + }); + + it('does not swallow unrelated 400s', async () => { + const provider = new AnthropicProvider({ apiKey: 'sk-ant-api-test' }); + (provider as unknown as { request: () => Promise }).request = async () => { + throw new Error('Anthropic HTTP 400: {"error":{"message":"max_tokens is required"}}'); + }; + await expect(provider.complete({ + model: 'claude-opus-5', + messages: [{ role: 'user', content: 'hi' }], + temperature: 0, + })).rejects.toThrow(/max_tokens/); + }); +});