From e2ef5bc6513179c162256b82c15f6dc300948b5a Mon Sep 17 00:00:00 2001 From: MisterWanted Date: Sun, 6 Sep 2026 20:43:01 +0200 Subject: [PATCH] fix: preserve tool names across bounded chat wire aliases Use one request-local reversible codec for declarations, forced choice and history. Reserve conforming names and resolve alias collisions with salted SHA-256 suffixes. Restore original names in JSON and streaming responses without changing call IDs or arguments. --- .../src/content/docs/reference/adapters.md | 4 + src/adapters/openai-chat-tool-names.ts | 56 ++++++++ src/adapters/openai-chat.ts | 15 ++- tests/adapter-usage.test.ts | 14 +- tests/openai-chat-tool-names.test.ts | 120 ++++++++++++++++++ 5 files changed, 202 insertions(+), 7 deletions(-) create mode 100644 src/adapters/openai-chat-tool-names.ts create mode 100644 tests/openai-chat-tool-names.test.ts diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 30a868f1e..943047e61 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -31,6 +31,10 @@ provider — xAI, Kimi, DeepSeek, GLM, Groq, OpenRouter, Ollama (local & cloud), - Converts internal messages to OpenAI roles; maps tools to `{type:"function", function:{…}}` and `tool_choice` (`auto`/`none`/`required` or a named function). +- Keeps function names within the 64-character ASCII wire limit using request-local, + collision-safe aliases. Declarations, named tool choice and conversation history share the + same mapping; streamed and non-streamed replies restore the original tool names without + changing call IDs or arguments. Already-valid names stay unchanged. - **Rewrites Codex's GPT-5 identity prompt** to a model-agnostic intro so routed models don't claim to be OpenAI. - **Clamps `reasoning_effort`** to the model's advertised subset when an exact tier is unavailable; diff --git a/src/adapters/openai-chat-tool-names.ts b/src/adapters/openai-chat-tool-names.ts new file mode 100644 index 000000000..03a4755f0 --- /dev/null +++ b/src/adapters/openai-chat-tool-names.ts @@ -0,0 +1,56 @@ +import { createHash } from "node:crypto"; + +const WIRE_NAME = /^[A-Za-z0-9_-]{1,64}$/; + +/** Request-local aliases: reserve valid originals before generating any aliases. */ +export function chatToolNameCodec(names: readonly string[]): { + toWire: (name: string) => string; + fromWire: (name: string) => string; +} { + const originals = [...new Set(names)]; + const used = new Set(originals.filter(name => WIRE_NAME.test(name))); + const encoded = new Map(); + const decoded = new Map(); + // Sorting also makes collision resolution independent of declaration/history order. + for (const name of originals.sort()) { + let wire = name; + if (!WIRE_NAME.test(name)) { + const prefix = (name.replace(/[^A-Za-z0-9_-]/g, "_") || "tool").slice(0, 55); + for (let salt = 0; ; salt++) { + const input = salt === 0 ? name : `${name}#${salt}`; + const suffix = createHash("sha256").update(input).digest("hex").slice(0, 8); + wire = `${prefix}_${suffix}`; + if (!used.has(wire)) break; + } + } + used.add(wire); + encoded.set(name, wire); + decoded.set(wire, name); + } + return { + toWire: name => encoded.get(name) ?? name, + fromWire: name => decoded.get(name) ?? name, + }; +} + +/** Rewrite only Chat function-name fields, never arguments, descriptions or call IDs. */ +export function encodeChatToolNames(messages: unknown[], tools: unknown[] | undefined, toolChoice: unknown): (name: string) => string { + const functions: Record[] = []; + const collect = (entry: unknown) => { + if (!entry || typeof entry !== "object") return; + const fn = (entry as { function?: unknown }).function; + if (fn && typeof fn === "object" && typeof (fn as { name?: unknown }).name === "string") { + functions.push(fn as Record); + } + }; + for (const tool of tools ?? []) collect(tool); + collect(toolChoice); + for (const message of messages) { + if (!message || typeof message !== "object") continue; + const calls = (message as { tool_calls?: unknown }).tool_calls; + if (Array.isArray(calls)) for (const call of calls) collect(call); + } + const codec = chatToolNameCodec(functions.map(fn => fn.name as string)); + for (const fn of functions) fn.name = codec.toWire(fn.name as string); + return codec.fromWire; +} diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index f52a7ee53..d5cfb390e 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -12,6 +12,7 @@ import { neutralizeIdentity } from "./identity"; import { buildNonOpenAIToolCatalogNudgeForTools, shouldInjectNonOpenAIToolCatalogNudge } from "./tool-catalog-nudge"; import { openRouterProviderPayload, resolveOpenRouterRouting } from "../providers/openrouter-routing"; import { resolveProviderCompat } from "../providers/compat"; +import { encodeChatToolNames } from "./openai-chat-tool-names"; // Providers may opt into stripping one trailing "[...]" group from the wire model id. // Z.AI needs this because its OpenAI path rejects glm-5.2[1m] with 400 code 1211; @@ -245,7 +246,7 @@ function messagesToChatFormat(parsed: OcxParsedRequest, provider: OcxProviderCon // WS turns can arrive with only tool outputs; chat-completions providers reject a bare // role:"tool" message unless an assistant tool_call with the same id immediately precedes it. flushPendingToolCalls(); - const name = safeToolName(msg.toolName); + const name = namespacedToolName(msg.toolNamespace, safeToolName(msg.toolName)); out.push({ role: "assistant", content: "", @@ -275,9 +276,8 @@ function messagesToChatFormat(parsed: OcxParsedRequest, provider: OcxProviderCon } function safeToolName(name: string | undefined): string { - const raw = name && name.trim().length > 0 ? name : "tool_result"; - const sanitized = raw.replace(/[^A-Za-z0-9_-]/g, "_"); - return sanitized; + // Encoding happens with the full request name set, so it remains reversible. + return name && name.trim().length > 0 ? name : "tool_result"; } const ZEN_SCHEMA_MAP_KEYS = new Set(["properties", "$defs", "definitions"]); @@ -614,6 +614,8 @@ function applyCompatThinkingFormat( } export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAdapter { + // resolveAdapter creates one adapter per request, as for Google/Kiro codecs. + let restoreToolName = (name: string): string => name; return { name: "openai-chat", @@ -628,6 +630,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const messages = messagesToChatFormat(parsed, provider); const tools = toolsToChatFormatForProvider(parsed, provider); const toolChoice = toolChoiceToChatFormat(parsed.options.toolChoice, parsed.context.tools); + restoreToolName = encodeChatToolNames(messages, tools, toolChoice); const body: Record = { model: provider.modelSuffixBracketStrip ? stripBracketedModelSuffix(parsed.modelId) : parsed.modelId, @@ -778,7 +781,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const flushToolCalls = function* (): Generator { for (const call of pendingToolCalls) { if (!call.id) call.id = `call_${++toolCallSeq}`; - yield { type: "tool_call_start", id: call.id, name: call.name }; + yield { type: "tool_call_start", id: call.id, name: restoreToolName(call.name) }; if (call.args.length > 0) yield { type: "tool_call_delta", arguments: call.args }; yield { type: "tool_call_end" }; } @@ -979,7 +982,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const toolCalls = msg.tool_calls as { id: string; function: { name: string; arguments: string } }[] | undefined; if (toolCalls) { for (const tc of toolCalls) { - events.push({ type: "tool_call_start", id: tc.id, name: tc.function.name }); + events.push({ type: "tool_call_start", id: tc.id, name: restoreToolName(tc.function.name) }); events.push({ type: "tool_call_delta", arguments: tc.function.arguments }); events.push({ type: "tool_call_end" }); } diff --git a/tests/adapter-usage.test.ts b/tests/adapter-usage.test.ts index b76c2dfe5..38c740b1b 100644 --- a/tests/adapter-usage.test.ts +++ b/tests/adapter-usage.test.ts @@ -372,7 +372,7 @@ describe("openai-chat tool history repair", () => { tool_calls: [{ id: "call_1", type: "function", - function: { name: "codex_list_mcp_resources", arguments: "{}" }, + function: { name: "codex_list_mcp_resources_9f17b48d", arguments: "{}" }, }], }); expect(body.messages[1]).toMatchObject({ @@ -380,6 +380,18 @@ describe("openai-chat tool history repair", () => { tool_call_id: "call_1", content: '{"resources":[]}', }); + const [call] = (body.messages[0].tool_calls as { + id: string; function: { name: string; arguments: string }; + }[]); + expect(call.function.name).toMatch(/^[A-Za-z0-9_-]{1,64}$/); + const restored = await adapter.parseResponse!(Response.json({ + choices: [{ message: { tool_calls: [call] } }], + })); + expect(restored.slice(0, 3)).toEqual([ + { type: "tool_call_start", id: "call_1", name: "codex.list_mcp_resources" }, + { type: "tool_call_delta", arguments: "{}" }, + { type: "tool_call_end" }, + ]); }); test("keeps paired tool results attached to the prior assistant tool_call", async () => { diff --git a/tests/openai-chat-tool-names.test.ts b/tests/openai-chat-tool-names.test.ts new file mode 100644 index 000000000..9c02a3189 --- /dev/null +++ b/tests/openai-chat-tool-names.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, test } from "bun:test"; +import { createOpenAIChatAdapter } from "../src/adapters/openai-chat"; +import { chatToolNameCodec } from "../src/adapters/openai-chat-tool-names"; +import { buildToolBridgeMaps } from "../src/server/responses/collaboration"; +import type { AdapterEvent, OcxParsedRequest } from "../src/types"; + +const longName = `search_${"a".repeat(80)}`; +const adapter = () => createOpenAIChatAdapter({ adapter: "openai-chat", baseUrl: "https://example.test/v1" }); +function request(namespace?: string): OcxParsedRequest { + return { + modelId: "test", stream: true, + options: { toolChoice: { name: namespace ? `${namespace}.${longName}` : longName } }, + context: { + tools: [{ name: longName, namespace, description: "test", parameters: { type: "object" } }], + messages: [ + { role: "assistant", timestamp: 0, content: [{ type: "toolCall", id: "call_original", name: longName, namespace, arguments: { name: longName } }] }, + { role: "toolResult", timestamp: 0, toolCallId: "call_original", toolName: longName, toolNamespace: namespace, content: "ok", isError: false }, + ], + }, + }; +} + +describe("Chat tool-name codec", () => { + test("preserves 64 valid ASCII characters and aliases 65, empty and invalid names", () => { + const names = ["a".repeat(64), "a".repeat(65), "with.dot/space é", "", "9-valid_name"]; + const codec = chatToolNameCodec(names); + expect(codec.toWire(names[0])).toBe(names[0]); + expect(codec.toWire(names[4])).toBe(names[4]); + for (const name of names) { + expect(codec.toWire(name)).toMatch(/^[A-Za-z0-9_-]{1,64}$/); + expect(codec.fromWire(codec.toWire(name))).toBe(name); + } + expect(codec.fromWire("unknown_tool")).toBe("unknown_tool"); + }); + + test("separates identical cleaned prefixes and reserves conforming originals regardless of order", () => { + const other = `${longName}b`; + const initial = chatToolNameCodec([longName]).toWire(longName); + const second = chatToolNameCodec([longName, initial]).toWire(longName); + // Force two consecutive salt candidates to collide with real, valid tool names. + const names = [longName, other, initial, second, longName]; + const codec = chatToolNameCodec(names); + const reversed = chatToolNameCodec([...names].reverse()); + expect(codec.toWire(initial)).toBe(initial); + expect(codec.toWire(second)).toBe(second); + expect(codec.toWire(longName)).not.toBe(initial); + expect(codec.toWire(longName)).not.toBe(second); + expect(new Set(names.map(name => codec.toWire(name))).size).toBe(4); + for (const name of names) { + expect(codec.fromWire(codec.toWire(name))).toBe(name); + expect(codec.toWire(name)).toBe(reversed.toWire(name)); + } + }); +}); + +describe("Chat adapter reversible tool-name wire contract", () => { + for (const namespace of [undefined, "mcp_namespace"]) { + test(`declaration, forced choice, history and JSON response agree (${namespace ?? "bare"})`, async () => { + const parsed = request(namespace); + const before = JSON.stringify(parsed); + const instance = adapter(); + const built = await instance.buildRequest(parsed); + const wire = JSON.parse(built.body as string); + const alias = wire.tools[0].function.name; + const history = wire.messages.filter((message: { role: string }) => message.role === "assistant" || message.role === "tool"); + expect(alias).toMatch(/^[A-Za-z0-9_-]{1,64}$/); + expect(wire.tool_choice.function.name).toBe(alias); + expect(history[0].tool_calls[0].function.name).toBe(alias); + expect(history[0].tool_calls[0].function.arguments).toBe(JSON.stringify({ name: longName })); + expect(history[0].tool_calls[0].id).toBe("call_original"); + expect(history[1].tool_call_id).toBe("call_original"); + expect(JSON.stringify(parsed)).toBe(before); + const events = await instance.parseResponse!(Response.json({ choices: [{ message: { tool_calls: [ + { id: "response_call", function: { name: alias, arguments: "{}" } }, + ] } }] })); + const start = events.find(event => event.type === "tool_call_start"); + const restored = namespace ? `${namespace}__${longName}` : longName; + expect(start).toEqual({ type: "tool_call_start", id: "response_call", name: restored }); + if (namespace) expect(buildToolBridgeMaps(parsed).toolNsMap.get(restored)).toEqual({ namespace, name: longName }); + }); + } + + test("streamed parallel calls retain ID association and restore bare/namespaced names", async () => { + const instance = adapter(); + const parsed = request("mcp"); + parsed.context.tools!.push({ name: "invalid.bare", description: "test", parameters: { type: "object" } }); + const built = await instance.buildRequest(parsed); + const wire = JSON.parse(built.body as string); + const calls = wire.tools.map((tool: { function: { name: string } }, index: number) => ({ index, id: `call_${index}`, function: { name: tool.function.name, arguments: "{" } })); + const frames = [ + { choices: [{ delta: { tool_calls: calls } }] }, + { choices: [{ delta: { tool_calls: [{ index: 1, function: { arguments: '"second":true}' } }, { index: 0, function: { arguments: '"first":true}' } }] }, finish_reason: "tool_calls" }] }, + ]; + const response = new Response(frames.map(frame => `data: ${JSON.stringify(frame)}\n\n`).join("") + "data: [DONE]\n\n"); + const events: AdapterEvent[] = []; + for await (const event of instance.parseStream(response)) events.push(event); + expect(events.filter(event => event.type === "tool_call_start")).toEqual([ + { type: "tool_call_start", id: "call_0", name: `mcp__${longName}` }, + { type: "tool_call_start", id: "call_1", name: "invalid.bare" }, + ]); + expect(events.filter(event => event.type === "tool_call_delta")).toEqual([ + { type: "tool_call_delta", arguments: '{"first":true}' }, + { type: "tool_call_delta", arguments: '{"second":true}' }, + ]); + }); + + test("orphan namespaced results and history-only names use the same bounded alias", async () => { + const parsed = request("mcp"); + parsed.context.messages.shift(); + const instance = adapter(); + const wire = JSON.parse((await instance.buildRequest(parsed)).body as string); + const history = wire.messages.filter((message: { role: string }) => message.role === "assistant" || message.role === "tool"); + expect(history[0].tool_calls[0].function.name).toBe(wire.tools[0].function.name); + expect(history[0].tool_calls[0].id).toBe(history[1].tool_call_id); + parsed.context.tools = []; + parsed.options = {}; + const historyOnly = JSON.parse((await instance.buildRequest(parsed)).body as string); + expect(historyOnly.messages[0].tool_calls[0].function.name).toMatch(/^[A-Za-z0-9_-]{1,64}$/); + }); +});