From bdc79ef883810c5c8656cca43726a544b2e11364 Mon Sep 17 00:00:00 2001 From: Xmon Dai Date: Fri, 18 Sep 2026 22:34:12 +0800 Subject: [PATCH] fix(providers): normalize non-function tools for allowlisted built-in OpenAI-format providers (#13855) Built-in providers that speak the OpenAI Chat wire format skipped normalizeOpenAICompatibleTools(), which only ran for custom openai-compatible-* connections. A client tool whose type is not "function" (a named Claude server tool, a nameless hosted tool) was forwarded verbatim, and agentrouter's GLM backend rejected the whole request with 400 tools[0].type:type is illegal. Extract the gate into shouldNormalizeFunctionToolsOnly(): custom openai-compatible-* providers keep normalizing on every target, and a conservative allowlist of built-in providers (agentrouter first) normalizes on the OpenAI Chat target only. OpenAI itself stays off the list, so its custom tools pass through untouched. Closes #13789 --- open-sse/handlers/chatCore.ts | 12 +- .../chatCore/openAICompatibleTools.ts | 35 +++- ...9-builtin-openai-nonfunction-tools.test.ts | 172 ++++++++++++++++++ tests/unit/openai-compatible-tools.test.ts | 24 +++ 4 files changed, 232 insertions(+), 11 deletions(-) create mode 100644 tests/unit/13789-builtin-openai-nonfunction-tools.test.ts diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index b9c275be1d..9447082f82 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -7,7 +7,10 @@ import { mergeInjectedFallbackOwnerNames, } from "./chatCore/memorySkillsInjection.ts"; import { resolveChatCoreRequestSetup } from "./chatCore/requestSetup.ts"; -import { normalizeOpenAICompatibleTools } from "./chatCore/openAICompatibleTools.ts"; +import { + normalizeOpenAICompatibleTools, + shouldNormalizeFunctionToolsOnly, +} from "./chatCore/openAICompatibleTools.ts"; import { buildFailureUsageRecord, projectFailureUsageErrorCode, @@ -2481,9 +2484,12 @@ export async function handleChatCore({ // This must happen before translateRequest, which validates and throws on unknown types. // Skip normalization when we are in native openai-compatible Responses passthrough mode // to preserve native tool definitions (exec with lark grammar, collaboration namespace, etc.). + // #13789: built-in providers observed to reject non-function tool types (agentrouter GLM: + // `400 tools[0].type:type is illegal`) are normalized too, via a conservative allowlist + // in shouldNormalizeFunctionToolsOnly that keeps openai's own `custom` tools untouched. if ( !nativeOpenAICompatibleResponsesPassthrough && - provider?.startsWith("openai-compatible-") && + shouldNormalizeFunctionToolsOnly(provider, targetFormat) && Array.isArray(translatedBody.tools) ) { const normalized = normalizeOpenAICompatibleTools( @@ -2495,7 +2501,7 @@ export async function handleChatCore({ if (dropped > 0) { log?.debug?.( "TOOLS", - `Dropped ${dropped} unconvertible tool(s) for openai-compatible provider` + `Dropped ${dropped} unconvertible tool(s) for ${provider} (function-tools-only)` ); } } diff --git a/open-sse/handlers/chatCore/openAICompatibleTools.ts b/open-sse/handlers/chatCore/openAICompatibleTools.ts index 3b692be36d..fa1579b49c 100644 --- a/open-sse/handlers/chatCore/openAICompatibleTools.ts +++ b/open-sse/handlers/chatCore/openAICompatibleTools.ts @@ -2,6 +2,31 @@ import { FORMATS } from "../../translator/formats.ts"; type Tool = Record; +// Built-in providers whose OpenAI Chat wire format was observed to accept only +// `type: "function"` tools. Keep this list conservative: only add a provider +// after a concrete upstream rejection of a non-function tool type (#13789, +// agentrouter GLM: `400 tools[0].type:type is illegal`). Providers that support +// richer tool types (openai `custom`, responses hosted tools) must NOT appear +// here — normalization would silently rewrite their requests. +const BUILTIN_FUNCTION_TOOLS_ONLY_PROVIDERS: ReadonlySet = new Set(["agentrouter"]); + +/** + * Whether non-function tool types must be converted/dropped before translation. + * + * Custom `openai-compatible-*` connections always normalize (they only support + * function tools). Built-in providers normalize only when on the allowlist AND + * the wire target is OpenAI Chat — the Responses target keeps native hosted + * tool definitions, and Claude targets have their own dispatch normalization. + */ +export function shouldNormalizeFunctionToolsOnly( + provider: string | undefined, + targetFormat: string +): boolean { + if (!provider) return false; + if (provider.startsWith("openai-compatible-")) return true; + return targetFormat === FORMATS.OPENAI && BUILTIN_FUNCTION_TOOLS_ONLY_PROVIDERS.has(provider); +} + export function normalizeOpenAICompatibleTools( tools: Tool[], sourceFormat: string @@ -15,17 +40,11 @@ export function normalizeOpenAICompatibleTools( const before = tools.length; const normalized = tools - .filter((tool) => - !tool.type || tool.type === "function" || !!tool.function || !!tool.name - ) + .filter((tool) => !tool.type || tool.type === "function" || !!tool.function || !!tool.name) .map((tool) => { // Responses custom tools carry free-form input. Preserve their native shape so // the Responses translator can produce the required { input: string } schema. - if ( - !tool.type || - tool.type === "function" || - tool.function - ) { + if (!tool.type || tool.type === "function" || tool.function) { return tool; } diff --git a/tests/unit/13789-builtin-openai-nonfunction-tools.test.ts b/tests/unit/13789-builtin-openai-nonfunction-tools.test.ts new file mode 100644 index 0000000000..cb4e6a0c50 --- /dev/null +++ b/tests/unit/13789-builtin-openai-nonfunction-tools.test.ts @@ -0,0 +1,172 @@ +// @ts-nocheck +// #13789 — built-in OpenAI-format providers forward non-function tool types unchanged. +// +// normalizeOpenAICompatibleTools() only ran for custom `openai-compatible-*` providers, +// so a built-in provider like agentrouter (OpenAI Chat wire) forwarded client tools such +// as `{ type: "web_search" }` verbatim and the GLM backend rejected the WHOLE request +// with `400 tools[0].type:type is illegal`. The fix extends the normalization to a +// conservative allowlist of built-in providers observed to accept function tools only, +// gated on the OpenAI Chat target format. OpenAI itself is NOT on the list, so its +// `custom` tools keep passing through untouched (acceptance criterion 2). +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-13789-nonfunction-tools-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const { handleChatCore } = await import("../../open-sse/handlers/chatCore.ts"); + +const originalFetch = globalThis.fetch; + +function noopLog() { + return { + debug() {}, + info() {}, + warn() {}, + error() {}, + }; +} + +function chatCompletionResponse(model) { + return new Response( + JSON.stringify({ + id: "chatcmpl_13789", + object: "chat.completion", + model, + choices: [ + { + index: 0, + message: { role: "assistant", content: "OK" }, + finish_reason: "stop", + }, + ], + usage: { prompt_tokens: 4, completion_tokens: 1, total_tokens: 5 }, + }), + { status: 200, headers: { "Content-Type": "application/json" } } + ); +} + +async function flushAsyncSideEffects() { + for (let i = 0; i < 5; i++) await new Promise((resolve) => setImmediate(resolve)); +} + +test.afterEach(async () => { + globalThis.fetch = originalFetch; + await flushAsyncSideEffects(); + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +}); + +test.after(() => { + globalThis.fetch = originalFetch; + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +async function runChatRequest({ provider, model, apiKey, tools }) { + let captured = null; + globalThis.fetch = async (url, init = {}) => { + captured = { + url: String(url), + body: JSON.parse(String(init.body || "{}")), + }; + return chatCompletionResponse(model); + }; + + const body = { + model: `${provider}/${model}`, + messages: [{ role: "user", content: "Reply with exactly OK" }], + max_tokens: 16, + stream: false, + tools, + }; + const result = await handleChatCore({ + body: structuredClone(body), + modelInfo: { provider, model, extendedContext: false }, + credentials: { apiKey, providerSpecificData: {} }, + log: noopLog(), + clientRawRequest: { + endpoint: "/v1/chat/completions", + body: structuredClone(body), + headers: new Headers({ accept: "application/json" }), + }, + userAgent: "test-client/1.0", + }); + return { result, captured }; +} + +test("#13789 agentrouter+glm: a request with non-function tools reaches the executor with none", async () => { + const { result, captured } = await runChatRequest({ + provider: "agentrouter", + model: "glm-5.3", + apiKey: "test-...ey", + tools: [ + { + type: "function", + function: { + name: "get_weather", + description: "Get weather", + parameters: { type: "object", properties: {} }, + }, + }, + // Acceptance shape from the issue: a hosted `{ type: "web_search" }` tool. + // The web-search fallback converts this to the omniroute_web_search function + // tool on this provider, so it never reaches upstream as a non-function type. + { type: "web_search" }, + // The shapes that actually leak on the current tip: a named Claude server + // tool (must convert to function) and a nameless hosted tool (must drop). + // Both forwarded verbatim, agentrouter GLM 400s the WHOLE request with + // `tools[0].type:type is illegal`. + { type: "computer_20241022", name: "computer", display_width_px: 1024 }, + { type: "code_interpreter" }, + ], + }); + + assert.equal(result.success, true); + assert.ok(captured, "the executor was reached"); + assert.equal(captured.url, "https://agentrouter.org/v1/chat/completions"); + + const tools = captured.body.tools ?? []; + for (const tool of tools) { + assert.equal( + tool.type, + "function", + `non-function tool reached agentrouter upstream: ${JSON.stringify(tool)}` + ); + } + const names = new Set(tools.map((t) => t.function?.name)); + assert.ok(names.has("get_weather"), "the plain function tool survives"); + assert.ok(names.has("computer"), "the named server tool is converted, not forwarded raw"); + assert.ok(names.has("omniroute_web_search"), "web_search lands as the fallback function tool"); + assert.equal( + names.has("code_interpreter"), + false, + "the nameless hosted tool is dropped, not forwarded" + ); +}); + +test("#13789 openai: `custom` tools stay untouched (allowlist is conservative)", async () => { + const customTool = { + type: "custom", + name: "exec", + format: { type: "grammar", grammar: "lark" }, + }; + const { result, captured } = await runChatRequest({ + provider: "openai", + model: "gpt-4o", + apiKey: "test-openai-key", + tools: [customTool], + }); + + assert.equal(result.success, true); + assert.ok(captured, "the executor was reached"); + const tools = captured.body.tools ?? []; + assert.equal(tools.length, 1); + assert.equal(tools[0].type, "custom", "openai keeps its own custom tool type"); + assert.equal(tools[0].name, "exec"); +}); diff --git a/tests/unit/openai-compatible-tools.test.ts b/tests/unit/openai-compatible-tools.test.ts index 5d00e2a69e..19243bf134 100644 --- a/tests/unit/openai-compatible-tools.test.ts +++ b/tests/unit/openai-compatible-tools.test.ts @@ -47,3 +47,27 @@ test("normalizes named non-function tools for other source formats", () => { dropped: 0, }); }); + +// #13789 — the normalization gate itself: which providers get non-function +// tools converted/dropped before translation. +const { shouldNormalizeFunctionToolsOnly } = + await import("../../open-sse/handlers/chatCore/openAICompatibleTools.ts"); + +test("#13789 custom openai-compatible-* providers always normalize", () => { + for (const format of [FORMATS.OPENAI, FORMATS.OPENAI_RESPONSES, FORMATS.CLAUDE]) { + assert.equal(shouldNormalizeFunctionToolsOnly("openai-compatible-acme", format), true); + } +}); + +test("#13789 allowlisted built-ins normalize on the OpenAI Chat target only", () => { + assert.equal(shouldNormalizeFunctionToolsOnly("agentrouter", FORMATS.OPENAI), true); + // Responses keeps native hosted tools; Claude has its own dispatch normalization. + assert.equal(shouldNormalizeFunctionToolsOnly("agentrouter", FORMATS.OPENAI_RESPONSES), false); + assert.equal(shouldNormalizeFunctionToolsOnly("agentrouter", FORMATS.CLAUDE), false); +}); + +test("#13789 every other provider (including openai) is untouched", () => { + for (const provider of ["openai", "anthropic", "gemini", "minimax", undefined]) { + assert.equal(shouldNormalizeFunctionToolsOnly(provider, FORMATS.OPENAI), false); + } +});