From 39cf76c11dd6ee56ca212335dcde3542375f3a0d Mon Sep 17 00:00:00 2001 From: Lukas <103962359+L4XB@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:32:18 +0200 Subject: [PATCH] fix(gemini): a tool name starting with a digit no longer fails the request (#13738) Google validates every `functionDeclarations[].name` against one grammar and rejects the WHOLE GenerateContentRequest when any single one is invalid, so six `1c_*` tools in a 109-tool MCP catalog made every request 400, including requests that would never call them (#13715). `normalizeGeminiToolName` removed invalid characters, collapsed underscores and stripped leading and trailing ones, none of which touches a leading digit. The strip is also why a leading underscore was not a workaround: the client's `_1c_probe` was normalized back to `1c_probe` and rejected for the same reason. The prefix goes inside the normalizer rather than at the call site, because that keeps the guarantee on the one value every path reads. `buildHashedGemini ToolName` builds its name from the normalized string and inherits its first character, so a fix applied later would hold for short names and fail silently for the long ones a 109-tool catalog is full of. It also runs before the collision check, so two names that newly collide are still separated by the existing hashed path. The reverse direction needs nothing: the sanitized name now differs from the client's, so `buildChangedToolNameMap` carries the original and the response translator restores the client's spelling on the model's functionCall. Six cells: no declared name starts with a digit, a leading underscore reaches Google letter-first, the reverse map returns the client's spelling, a letter-first name is untouched, an over-long digit-first name keeps the guarantee through the hash path, and two colliding digit-first names stay distinct. Four mutations, all killed. --- .../helpers/geminiToolsSanitizer.ts | 19 +++- ...715-gemini-tool-name-leading-digit.test.ts | 101 ++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 tests/unit/13715-gemini-tool-name-leading-digit.test.ts diff --git a/open-sse/translator/helpers/geminiToolsSanitizer.ts b/open-sse/translator/helpers/geminiToolsSanitizer.ts index 9e2979d1fe..7d93c36590 100644 --- a/open-sse/translator/helpers/geminiToolsSanitizer.ts +++ b/open-sse/translator/helpers/geminiToolsSanitizer.ts @@ -60,10 +60,21 @@ function normalizeGeminiToolName( return namespaceIndex >= 0 ? trimmed.slice(namespaceIndex + 1) : trimmed; })(); - return namespaceStripped - .replace(/[^a-zA-Z0-9_]/g, "_") - .replace(/_+/g, "_") - .replace(/^_+|_+$/g, ""); + return ( + namespaceStripped + .replace(/[^a-zA-Z0-9_]/g, "_") + .replace(/_+/g, "_") + .replace(/^_+|_+$/g, "") + // Google rejects the WHOLE GenerateContentRequest when any one + // functionDeclaration name fails its grammar, and that grammar requires a + // letter or an underscore first. The strip above has just removed any + // leading underscore, so a name like `1c_plugin_reload` reached Google + // unchanged and took the other 108 tools down with it (#13715). Prefixing + // here rather than at the call site keeps the guarantee on the one value + // every path reads: the length cap and the collision hash both build on + // this string, and a hashed name inherits its first character from it. + .replace(/^(\d)/, "t$1") + ); } function buildHashedGeminiToolName( diff --git a/tests/unit/13715-gemini-tool-name-leading-digit.test.ts b/tests/unit/13715-gemini-tool-name-leading-digit.test.ts new file mode 100644 index 0000000000..5346f87510 --- /dev/null +++ b/tests/unit/13715-gemini-tool-name-leading-digit.test.ts @@ -0,0 +1,101 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { buildGeminiTools, sanitizeGeminiToolName } = + await import("../../open-sse/translator/helpers/geminiToolsSanitizer.ts"); +const { buildChangedToolNameMap } = + await import("../../open-sse/translator/request/openai-to-gemini/helpers.ts"); + +// ── Gemini rejects a function name that does not start with a letter (#13715) ── +// +// Google validates every `functionDeclarations[].name` against one grammar and +// fails the WHOLE GenerateContentRequest when any single one is invalid, so six +// `1c_*` tools in a 109-tool MCP catalog made every request 400 -- including +// requests that would never call them. The sanitizer removed invalid characters +// and stripped leading underscores, which left a leading DIGIT untouched, and +// stripped away the one prefix a user could have added by hand. + +const fn = (name: string) => ({ + type: "function", + function: { name, description: "probe", parameters: { type: "object", properties: {} } }, +}); + +const declaredNames = (tools: unknown[], toolNameMap: Map) => + (buildGeminiTools(tools, { toolNameMap }) ?? []).flatMap( + (tool) => tool.functionDeclarations?.map((declaration) => declaration.name) ?? [] + ); + +test("#13715 a declared name never starts with a digit", () => { + const toolNameMap = new Map(); + const names = declaredNames( + [fn("1c_probe"), fn("1c_ssl_mcp_plugin_reload"), fn("123"), fn("_1c_probe")], + toolNameMap + ); + + for (const name of names) { + assert.match(name, /^[A-Za-z_]/, `"${name}" must start with a letter or an underscore`); + } + assert.deepEqual(names.slice(0, 3), ["t1c_probe", "t1c_ssl_mcp_plugin_reload", "t123"]); + // `_1c_probe` and `1c_probe` are two different client tools that normalize to + // the same name, so the fourth is a real collision and the existing hashed + // path separates them rather than one overwriting the other's mapping. + assert.notEqual(names[3], names[0]); + assert.match(names[3]!, /^t1c_probe_/); +}); + +test("#13715 a leading underscore is not a workaround, because it is stripped first", () => { + // The measured row from the report: the client sending `_1c_probe` got the + // same 400, because the strip turned it back into `1c_probe`. It must now + // reach Google as a letter-first name like every other spelling. + const toolNameMap = new Map(); + assert.equal(sanitizeGeminiToolName("_1c_probe", { toolNameMap }), "t1c_probe"); +}); + +test("#13715 the client's own spelling comes back on the reverse map", () => { + // The half that makes the rename invisible to the caller: the sanitized name + // now differs from the client's, so the map carries the original and the + // response translator restores it when the model calls the tool. + const toolNameMap = new Map(); + sanitizeGeminiToolName("1c_ssl_mcp_plugin_reload", { toolNameMap }); + + const reverse = buildChangedToolNameMap(toolNameMap); + assert.equal(reverse?.get("t1c_ssl_mcp_plugin_reload"), "1c_ssl_mcp_plugin_reload"); +}); + +test("#13715 a name that already starts with a letter is untouched", () => { + // The accept control. Prefixing unconditionally would rename every tool in + // every catalog and put the whole fleet through the reverse map for nothing. + const toolNameMap = new Map(); + for (const name of ["c1_probe", "_abc", "Bash", "ssl.probe", "ssl-probe"]) { + const sanitized = sanitizeGeminiToolName(name, { toolNameMap }); + assert.doesNotMatch(sanitized, /^t(?=[0-9])/, `"${name}" must not gain a prefix`); + } + assert.equal(sanitizeGeminiToolName("c1_probe", { toolNameMap: new Map() }), "c1_probe"); +}); + +test("#13715 an over-long digit-first name keeps the guarantee through the hash path", () => { + // The hashed name is built FROM the normalized one, so a fix applied at the + // call site instead of inside the normalizer would hold for short names and + // silently fail for long ones, which is the shape a 109-tool catalog has. + const toolNameMap = new Map(); + const long = `1c_${"x".repeat(80)}`; + const sanitized = sanitizeGeminiToolName(long, { toolNameMap }); + + assert.ok(sanitized.length <= 64, `"${sanitized}" exceeds the 64 character cap`); + assert.match(sanitized, /^[A-Za-z_]/); + assert.equal(toolNameMap.get(sanitized), long); +}); + +test("#13715 two digit-first names that collide still resolve to distinct declarations", () => { + // The prefix runs before the collision check, so the existing hashed-name path + // still separates them rather than one tool overwriting the other's mapping. + const toolNameMap = new Map(); + const first = sanitizeGeminiToolName("1c:probe", { toolNameMap, stripNamespace: false }); + const second = sanitizeGeminiToolName("1c_probe", { toolNameMap, stripNamespace: false }); + + assert.notEqual(first, second); + assert.match(first, /^[A-Za-z_]/); + assert.match(second, /^[A-Za-z_]/); + assert.equal(toolNameMap.get(first), "1c:probe"); + assert.equal(toolNameMap.get(second), "1c_probe"); +});