mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 13:23:50 +03:00
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
This commit is contained in:
@@ -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)`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,31 @@ import { FORMATS } from "../../translator/formats.ts";
|
||||
|
||||
type Tool = Record<string, unknown>;
|
||||
|
||||
// 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<string> = 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;
|
||||
}
|
||||
|
||||
|
||||
172
tests/unit/13789-builtin-openai-nonfunction-tools.test.ts
Normal file
172
tests/unit/13789-builtin-openai-nonfunction-tools.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
@@ -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);
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user