From ebc92194bb9bf2810e10f64de1fa6e599c2fa156 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 18 Apr 2026 19:39:13 -0400 Subject: [PATCH] fix(proxy): harden Anthropic request transformation semantics - enforce strict tool_result ordering and pairing against assistant tool_use ids - reject tool_result image payloads that cannot map to OpenAI tool messages - preserve raw tool schemas on the /v1/messages proxy path instead of silently tightening them - forward Anthropic tool_choice semantics and cover adaptive routing plus upstream payload checks --- src/proxy/transformers/request-transformer.ts | 198 ++++++++++++++---- .../proxy/messages-endpoint.test.ts | 68 ++++++ .../integration/proxy/request-routing.test.ts | 71 +++++++ .../request-transformer-regressions.test.ts | 133 +++++++++++- 4 files changed, 422 insertions(+), 48 deletions(-) diff --git a/src/proxy/transformers/request-transformer.ts b/src/proxy/transformers/request-transformer.ts index c4547601..c1ebd5d7 100644 --- a/src/proxy/transformers/request-transformer.ts +++ b/src/proxy/transformers/request-transformer.ts @@ -1,5 +1,3 @@ -import { normalizeSchemaForOpenAI } from '../../utils/schema-sanitizer'; - interface AnthropicThinking { type?: 'enabled' | 'disabled' | 'adaptive' | string; budget_tokens?: number; @@ -50,6 +48,12 @@ interface AnthropicOutputConfig { effort?: 'low' | 'medium' | 'high' | 'max' | string; } +interface AnthropicToolChoice { + type?: 'auto' | 'any' | 'tool' | 'none' | string; + name?: string; + disable_parallel_tool_use?: boolean; +} + interface AnthropicProxyRequestShape { model?: unknown; system?: unknown; @@ -60,6 +64,7 @@ interface AnthropicProxyRequestShape { stop_sequences?: unknown; metadata?: unknown; tools?: unknown; + tool_choice?: AnthropicToolChoice; stream?: unknown; thinking?: AnthropicThinking; output_config?: AnthropicOutputConfig; @@ -109,6 +114,17 @@ export interface ProxyOpenAIRequest { parameters: Record; }; }>; + tool_choice?: + | 'auto' + | 'none' + | 'required' + | { + type: 'function'; + function: { + name: string; + }; + }; + parallel_tool_calls?: boolean; messages: OpenAIMessage[]; max_tokens?: number; temperature?: number; @@ -181,10 +197,7 @@ function flattenTextContent(content: unknown, label: string): string { * Handles strings, arrays with text/image blocks, and error prefixing. * Ported from openclaude's convertToolResultContent. */ -function convertToolResultContent( - content: unknown, - isError: boolean -): string | OpenAIContentPart[] { +function convertToolResultContent(content: unknown, isError: boolean, label: string): string { if (content === undefined) { return ''; } @@ -196,43 +209,32 @@ function convertToolResultContent( return isError ? `Error: ${text}` : text; } - const parts: OpenAIContentPart[] = []; - for (const block of content) { - if (block?.type === 'text' && typeof block.text === 'string') { - parts.push({ type: 'text', text: block.text }); + const parts: string[] = []; + for (const [index, block] of content.entries()) { + const parsed = assertObject(block, `${label}[${index}]`); + + if (parsed.type === 'text' && typeof parsed.text === 'string') { + parts.push(parsed.text); continue; } - if (block?.type === 'image') { - const source = block.source; - if (source?.type === 'url' && source.url) { - parts.push({ type: 'image_url', image_url: { url: source.url } }); - } else if (source?.type === 'base64' && source.media_type && source.data) { - parts.push({ - type: 'image_url', - image_url: { url: `data:${source.media_type};base64,${source.data}` }, - }); - } + if (parsed.type === 'image') { + throw new Error(`${label}[${index}].type "image" is not supported in tool_result content`); + } + + if (typeof parsed.text === 'string') { + parts.push(parsed.text); continue; } - if (typeof block?.text === 'string') { - parts.push({ type: 'text', text: block.text }); - } + throw new Error(`${label}[${index}].type "${String(parsed.type)}" is not supported`); } - if (parts.length === 0) return ''; - if (parts.length === 1 && parts[0].type === 'text') { - const text = (parts[0] as OpenAITextPart).text; - return isError ? `Error: ${text}` : text; + const text = parts.join('\n'); + if (!text) { + return isError ? 'Error:' : ''; } - if (isError && parts[0]?.type === 'text') { - parts[0] = { ...parts[0], text: `Error: ${(parts[0] as OpenAITextPart).text}` }; - } else if (isError) { - parts.unshift({ type: 'text', text: 'Error:' }); - } - - return parts; + return isError ? `Error: ${text}` : text; } function createFallbackToolId(messageIndex: number, blockIndex: number): string { @@ -310,7 +312,7 @@ function transformTools(value: unknown): ProxyOpenAIRequest['tools'] { function: { name: typeof entry.name === 'string' ? entry.name : 'tool', ...(typeof entry.description === 'string' ? { description: entry.description } : {}), - parameters: normalizeSchemaForOpenAI(rawSchema), + parameters: rawSchema, }, }; }); @@ -318,6 +320,45 @@ function transformTools(value: unknown): ProxyOpenAIRequest['tools'] { return tools.length > 0 ? tools : undefined; } +function transformToolChoice( + value: AnthropicToolChoice | undefined, + hasTools: boolean +): Pick { + if (!value) { + return hasTools ? { tool_choice: 'auto' } : {}; + } + + if (!hasTools) { + throw new Error('tool_choice requires tools'); + } + + const parallelToolCalls = + value.disable_parallel_tool_use === true ? { parallel_tool_calls: false } : {}; + + switch (value.type) { + case undefined: + case 'auto': + return { tool_choice: 'auto', ...parallelToolCalls }; + case 'none': + return { tool_choice: 'none' }; + case 'any': + return { tool_choice: 'required', ...parallelToolCalls }; + case 'tool': + if (typeof value.name !== 'string' || value.name.trim().length === 0) { + throw new Error('tool_choice.name must be a non-empty string when type is "tool"'); + } + return { + tool_choice: { + type: 'function', + function: { name: value.name.trim() }, + }, + ...parallelToolCalls, + }; + default: + throw new Error('tool_choice.type must be "auto", "any", "tool", or "none"'); + } +} + function mapThinkingToReasoning( thinking: AnthropicThinking | undefined, outputConfig: AnthropicOutputConfig | undefined @@ -384,6 +425,8 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { } const translatedMessages: OpenAIMessage[] = []; + let pendingToolUseIds: Set | null = null; + let hasPendingToolUseIds = false; messagesValue.forEach((message, messageIndex) => { const parsedMessage = assertObject(message, `messages[${messageIndex}]`) as AnthropicMessage; @@ -392,8 +435,19 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { throw new Error(`messages[${messageIndex}].role must be "user" or "assistant"`); } + if (pendingToolUseIds && pendingToolUseIds.size > 0 && role !== 'user') { + throw new Error( + `messages[${messageIndex}].role must be "user" with tool_result blocks after assistant tool_use` + ); + } + const content = parsedMessage.content; if (typeof content === 'string') { + if (pendingToolUseIds && pendingToolUseIds.size > 0) { + throw new Error( + `messages[${messageIndex}].content must start with tool_result blocks for pending tool_use ids` + ); + } translatedMessages.push({ role, content }); return; } @@ -405,6 +459,7 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { if (role === 'user') { const userParts: OpenAIContentPart[] = []; let sawToolResult = false; + const resolvedToolUseIds = new Set(); content.forEach((block, blockIndex) => { const parsed = assertObject( @@ -417,28 +472,62 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { } if (parsed.type === 'text') { + if (sawToolResult) { + throw new Error( + `messages[${messageIndex}].content[${blockIndex}] text is not allowed after tool_result blocks` + ); + } const text = typeof parsed.text === 'string' ? parsed.text : ''; userParts.push({ type: 'text', text }); return; } if (isImageBlock(parsed)) { + if (sawToolResult) { + throw new Error( + `messages[${messageIndex}].content[${blockIndex}] image is not allowed after tool_result blocks` + ); + } userParts.push(toImagePart(parsed, `messages[${messageIndex}].content[${blockIndex}]`)); return; } if (isToolResultBlock(parsed)) { + if (!pendingToolUseIds || pendingToolUseIds.size === 0) { + throw new Error( + `messages[${messageIndex}].content[${blockIndex}] tool_result requires a preceding assistant tool_use` + ); + } + if (userParts.length > 0) { + throw new Error( + `messages[${messageIndex}].content[${blockIndex}] tool_result blocks must come before other user content` + ); + } if (typeof parsed.tool_use_id !== 'string' || parsed.tool_use_id.trim().length === 0) { throw new Error( `messages[${messageIndex}].content[${blockIndex}].tool_use_id must be a non-empty string` ); } + if (!pendingToolUseIds.has(parsed.tool_use_id)) { + throw new Error( + `messages[${messageIndex}].content[${blockIndex}].tool_use_id "${parsed.tool_use_id}" does not match a pending tool_use` + ); + } + if (resolvedToolUseIds.has(parsed.tool_use_id)) { + throw new Error( + `messages[${messageIndex}].content[${blockIndex}].tool_use_id "${parsed.tool_use_id}" is duplicated` + ); + } sawToolResult = true; - flushUserContent(translatedMessages, userParts); + resolvedToolUseIds.add(parsed.tool_use_id); translatedMessages.push({ role: 'tool', tool_call_id: parsed.tool_use_id, - content: convertToolResultContent(parsed.content, parsed.is_error === true), + content: convertToolResultContent( + parsed.content, + parsed.is_error === true, + `messages[${messageIndex}].content[${blockIndex}].content` + ), }); return; } @@ -454,7 +543,24 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { ); }); - if (userParts.length > 0 || !sawToolResult) { + if (sawToolResult) { + if (resolvedToolUseIds.size !== pendingToolUseIds?.size) { + throw new Error( + `messages[${messageIndex}].content must provide tool_result blocks for all pending tool_use ids` + ); + } + pendingToolUseIds = null; + hasPendingToolUseIds = false; + return; + } + + if (pendingToolUseIds && pendingToolUseIds.size > 0) { + throw new Error( + `messages[${messageIndex}].content must start with tool_result blocks for pending tool_use ids` + ); + } + + if (userParts.length > 0) { flushUserContent(translatedMessages, userParts); } return; @@ -506,12 +612,20 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { `messages[${messageIndex}].content[${blockIndex}] tool_result requires user role` ); } + + throw new Error( + `messages[${messageIndex}].content[${blockIndex}].type "${String(parsed.type)}" is not supported` + ); }); if (assistantTextParts.length === 0 && toolCalls.length === 0) { return; } + pendingToolUseIds = + toolCalls.length > 0 ? new Set(toolCalls.map((toolCall) => toolCall.id)) : null; + hasPendingToolUseIds = toolCalls.length > 0; + translatedMessages.push({ role: 'assistant', content: assistantTextParts.join('\n'), @@ -519,6 +633,10 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { }); }); + if (hasPendingToolUseIds) { + throw new Error('messages must provide tool_result blocks for the latest assistant tool_use'); + } + return translatedMessages; } @@ -565,6 +683,7 @@ function coalesceMessages(messages: OpenAIMessage[]): OpenAIMessage[] { export class ProxyRequestTransformer { transform(raw: unknown): ProxyOpenAIRequest { const source = assertObject(raw || {}, 'request') as AnthropicProxyRequestShape; + const tools = transformTools(source.tools); const messages = transformMessages(source.messages); const system = source.system; const allMessages = @@ -587,7 +706,8 @@ export class ProxyRequestTransformer { top_p: asNumber(source.top_p), stop: asStringArray(source.stop_sequences), metadata: asMetadata(source.metadata), - tools: transformTools(source.tools), + tools, + ...transformToolChoice(source.tool_choice, tools !== undefined), ...mapThinkingToReasoning(source.thinking, source.output_config), }; } diff --git a/tests/integration/proxy/messages-endpoint.test.ts b/tests/integration/proxy/messages-endpoint.test.ts index d6ce7469..0c3fe0d0 100644 --- a/tests/integration/proxy/messages-endpoint.test.ts +++ b/tests/integration/proxy/messages-endpoint.test.ts @@ -116,13 +116,63 @@ describe('openai proxy messages endpoint', () => { const parsedUpstream = upstreamBody as { messages?: Array<{ role: string; content: string }>; + tool_choice?: unknown; tools?: Array<{ type: string; function: { name: string } }>; }; expect(parsedUpstream.messages?.[0]).toEqual({ role: 'user', content: 'Find docs' }); + expect(parsedUpstream.tool_choice).toBe('auto'); expect(parsedUpstream.tools?.[0]?.type).toBe('function'); expect(parsedUpstream.tools?.[0]?.function.name).toBe('search'); }); + it('preserves tool schemas and forwards explicit tool_choice semantics upstream', async () => { + const response = await requestProxy({ + model: 'hf-model', + messages: [{ role: 'user', content: [{ type: 'text', text: 'Search docs' }] }], + tools: [ + { + name: 'search', + description: 'Search docs', + input_schema: { + type: 'object', + properties: { + q: { type: 'string', pattern: '^[a-z]+$' }, + }, + required: ['q'], + additionalProperties: true, + }, + }, + ], + tool_choice: { + type: 'tool', + name: 'search', + disable_parallel_tool_use: true, + }, + }); + + expect(response.status).toBe(200); + + const parsedUpstream = upstreamBody as { + tool_choice?: unknown; + parallel_tool_calls?: boolean; + tools?: Array<{ type: string; function: { parameters: Record } }>; + }; + + expect(parsedUpstream.tool_choice).toEqual({ + type: 'function', + function: { name: 'search' }, + }); + expect(parsedUpstream.parallel_tool_calls).toBe(false); + expect(parsedUpstream.tools?.[0]?.function.parameters).toEqual({ + type: 'object', + properties: { + q: { type: 'string', pattern: '^[a-z]+$' }, + }, + required: ['q'], + additionalProperties: true, + }); + }); + it('falls back to Anthropic JSON for non-streaming requests', async () => { const response = await requestProxy({ model: 'hf-model', @@ -155,6 +205,24 @@ describe('openai proxy messages endpoint', () => { expect(body.error?.message).toContain('Invalid JSON'); }); + it('returns invalid_request_error for orphan tool_result blocks', async () => { + const response = await requestProxy({ + model: 'hf-model', + messages: [ + { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_orphan', content: 'orphan' }], + }, + ], + }); + + const body = (await response.json()) as { error?: { type?: string; message?: string } }; + + expect(response.status).toBe(400); + expect(body.error?.type).toBe('invalid_request_error'); + expect(body.error?.message).toContain('tool_result requires a preceding assistant tool_use'); + }); + it('rejects requests without the local proxy auth token', async () => { const response = await fetch(`http://127.0.0.1:${proxyPort}/v1/messages`, { method: 'POST', diff --git a/tests/integration/proxy/request-routing.test.ts b/tests/integration/proxy/request-routing.test.ts index d5a8ce0d..b9f7afe8 100644 --- a/tests/integration/proxy/request-routing.test.ts +++ b/tests/integration/proxy/request-routing.test.ts @@ -214,4 +214,75 @@ describe('openai proxy request routing', () => { expect(hits).toEqual(['thinker']); expect(bodies[0]?.body).toMatchObject({ model: 'deepseek-reasoner' }); }); + + it('routes adaptive thinking requests through the configured think scenario', async () => { + const primaryPort = await getPort(); + const thinkPort = await getPort(); + const hits: string[] = []; + const bodies: Array<{ label: string; body: unknown }> = []; + await startMockUpstream(primaryPort, 'primary', hits, bodies); + await startMockUpstream(thinkPort, 'thinker', hits, bodies); + + const primarySettings = writeSettings('hf', { + ANTHROPIC_BASE_URL: `http://127.0.0.1:${primaryPort}`, + ANTHROPIC_AUTH_TOKEN: 'hf_token', + ANTHROPIC_MODEL: 'hf-default', + CCS_DROID_PROVIDER: 'generic-chat-completion-api', + }); + const thinkSettings = writeSettings('thinker', { + ANTHROPIC_BASE_URL: `http://127.0.0.1:${thinkPort}`, + ANTHROPIC_AUTH_TOKEN: 'think_token', + ANTHROPIC_MODEL: 'deepseek-reasoner', + CCS_DROID_PROVIDER: 'generic-chat-completion-api', + }); + + fs.writeFileSync( + path.join(tempDir, '.ccs', 'config.json'), + JSON.stringify( + { + profiles: { hf: primarySettings, thinker: thinkSettings }, + proxy: { + routing: { + think: 'thinker:deepseek-reasoner', + }, + }, + }, + null, + 2 + ), + 'utf8' + ); + + const profile: OpenAICompatProfileConfig = { + profileName: 'hf', + settingsPath: primarySettings, + baseUrl: `http://127.0.0.1:${primaryPort}`, + apiKey: 'hf_token', + provider: 'generic-chat-completion-api', + model: 'hf-default', + }; + proxyServer = startOpenAICompatProxyServer({ + profile, + port: proxyPort, + authToken: 'test-proxy-token', + }); + + const response = await requestProxy({ + model: 'hf-default', + thinking: { type: 'adaptive' }, + output_config: { effort: 'max' }, + messages: [{ role: 'user', content: 'think adaptively' }], + }); + + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + content: [{ type: 'text', text: 'Reply from thinker' }], + }); + expect(hits).toEqual(['thinker']); + expect(bodies[0]?.body).toMatchObject({ + model: 'deepseek-reasoner', + reasoning_effort: 'high', + reasoning: { enabled: true, effort: 'high' }, + }); + }); }); diff --git a/tests/unit/proxy/transformers/request-transformer-regressions.test.ts b/tests/unit/proxy/transformers/request-transformer-regressions.test.ts index b4c232c8..e9a05c18 100644 --- a/tests/unit/proxy/transformers/request-transformer-regressions.test.ts +++ b/tests/unit/proxy/transformers/request-transformer-regressions.test.ts @@ -74,8 +74,114 @@ describe('ProxyRequestTransformer regressions', () => { ).toThrow('tool_result requires user role'); }); - it('translates url images and error tool results while coalescing repeated turns', () => { + it('rejects orphaned, incomplete, or mixed-order tool_result blocks', () => { + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_1', content: 'orphan' }], + }, + ], + }) + ).toThrow('tool_result requires a preceding assistant tool_use'); + + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [ + { type: 'tool_use', id: 'toolu_1', name: 'search', input: { q: 'docs' } }, + { type: 'tool_use', id: 'toolu_2', name: 'open', input: { url: 'https://example.com' } }, + ], + }, + { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_1', content: 'partial' }], + }, + ], + }) + ).toThrow('must provide tool_result blocks for all pending tool_use ids'); + + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [{ type: 'tool_use', id: 'toolu_1', name: 'vision', input: { detail: 'high' } }], + }, + { + role: 'user', + content: [ + { type: 'text', text: 'Here you go' }, + { type: 'tool_result', tool_use_id: 'toolu_1', content: 'result' }, + ], + }, + ], + }) + ).toThrow('tool_result blocks must come before other user content'); + + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [{ type: 'tool_use', id: 'toolu_1', name: 'vision', input: { detail: 'high' } }], + }, + { + role: 'user', + content: 'plain follow-up', + }, + ], + }) + ).toThrow('must start with tool_result blocks for pending tool_use ids'); + }); + + it('rejects tool_result content that cannot be represented as OpenAI tool text', () => { + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [{ type: 'tool_use', id: 'toolu_1', name: 'vision', input: { detail: 'high' } }], + }, + { + role: 'user', + content: [ + { + type: 'tool_result', + tool_use_id: 'toolu_1', + content: [{ type: 'image', source: { type: 'url', url: 'https://example.com/error.png' } }], + }, + ], + }, + ], + }) + ).toThrow('type "image" is not supported in tool_result content'); + }); + + it('rejects unsupported assistant blocks instead of silently dropping them', () => { + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [{ type: 'server_tool_use', id: 'srv_1' }], + }, + ], + }) + ).toThrow('type "server_tool_use" is not supported'); + }); + + it('translates url images and tool_choice while coalescing repeated turns', () => { const result = new ProxyRequestTransformer().transform({ + tool_choice: { + type: 'tool', + name: 'vision', + disable_parallel_tool_use: true, + }, + tools: [{ name: 'vision', description: 'Inspect image', input_schema: { type: 'object' } }], messages: [ { role: 'user', @@ -97,16 +203,19 @@ describe('ProxyRequestTransformer regressions', () => { type: 'tool_result', tool_use_id: 'toolu_1', is_error: true, - content: [ - { type: 'text', text: 'fetch failed' }, - { type: 'image', source: { type: 'url', url: 'https://example.com/error.png' } }, - ], + content: [{ type: 'text', text: 'fetch failed' }], }, ], }, ], }); + expect(result.tool_choice).toEqual({ + type: 'function', + function: { name: 'vision' }, + }); + expect(result.parallel_tool_calls).toBe(false); + expect(result.messages[0]).toEqual({ role: 'user', content: [ @@ -131,10 +240,16 @@ describe('ProxyRequestTransformer regressions', () => { expect(result.messages[2]).toEqual({ role: 'tool', tool_call_id: 'toolu_1', - content: [ - { type: 'text', text: 'Error: fetch failed' }, - { type: 'image_url', image_url: { url: 'https://example.com/error.png' } }, - ], + content: 'Error: fetch failed', }); }); + + it('defaults tools to auto tool_choice when none is specified', () => { + const result = new ProxyRequestTransformer().transform({ + messages: [{ role: 'user', content: 'hello' }], + tools: [{ name: 'search', description: 'Search docs', input_schema: { type: 'object' } }], + }); + + expect(result.tool_choice).toBe('auto'); + }); });