fix(translator): sanitize tool names for Gemini function_declarations compatibility#2268
Conversation
… compatibility Claude Code and MCP clients may send tool names containing characters invalid for Gemini's function_declarations (e.g. '/', '@', spaces). Sanitize on request via SanitizeFunctionName and restore original names on response for both antigravity/claude and gemini-cli/claude translators.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a mechanism to sanitize tool names in requests to ensure compatibility with Gemini's Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a critical compatibility issue by sanitizing tool names for Gemini API requests and restoring them in responses. The implementation is well-structured and consistently applied across both the antigravity and gemini-cli translators. I've identified a potential issue with how sanitized name collisions are handled, an opportunity to improve test coverage for this edge case, and an inefficiency in the streaming response handler for antigravity/claude. My detailed comments provide suggestions to address these points.
| if _, exists := out[sanitized]; !exists { | ||
| out[sanitized] = name | ||
| } |
There was a problem hiding this comment.
This logic silently ignores sanitized tool name collisions. If two different tool names (e.g., tool/a and tool@a) sanitize to the same value (tool_a), only the first one encountered will be stored in the reverse map. This will lead to incorrect tool name restoration for any subsequent tools that collide. While the upstream API would likely reject the request due to duplicate function names, it would be better to handle this collision explicitly, for example by logging a warning to aid in debugging.
| if params.ToolNameMap == nil { | ||
| params.ToolNameMap = util.SanitizedToolNameMap(originalRequestRawJSON) | ||
| } |
There was a problem hiding this comment.
This lazy initialization can be inefficient. SanitizedToolNameMap returns nil if no tool names need sanitization. In that case, params.ToolNameMap remains nil, causing SanitizedToolNameMap to be called and the request JSON to be parsed on every single streaming chunk. A better approach is to ensure this initialization logic runs only once. This can be achieved by adding a boolean flag (e.g., toolNameMapInitialized) to the Params struct and updating this logic accordingly.
| if params.ToolNameMap == nil { | |
| params.ToolNameMap = util.SanitizedToolNameMap(originalRequestRawJSON) | |
| } | |
| if !params.toolNameMapInitialized { | |
| params.ToolNameMap = util.SanitizedToolNameMap(originalRequestRawJSON) | |
| params.toolNameMapInitialized = true | |
| } |
| } | ||
| } | ||
|
|
||
| func TestSanitizedToolNameMap(t *testing.T) { |
There was a problem hiding this comment.
It would be beneficial to add a test case to TestSanitizedToolNameMap that covers the scenario of a sanitized tool name collision. For example, providing two different tool names in the input JSON that produce the same sanitized name (e.g., mcp/server/read and mcp-server-read). This would help verify and document the behavior for this edge case, where the current implementation silently keeps the first mapping.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2398ebad55
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if _, exists := out[sanitized]; !exists { | ||
| out[sanitized] = name |
There was a problem hiding this comment.
In SanitizedToolNameMap, a sanitized key is stored only once (if _, exists := out[sanitized]; !exists), so colliding names are silently merged. I checked both updated response paths (antigravity and gemini-cli), and they now rely on this map via RestoreSanitizedToolName; with tools like read_file and read/file (or any two names that sanitize to the same token), a provider functionCall.name of read_file can be restored to the wrong original tool name, causing the client to execute a different tool than the model intended. This needs explicit collision handling (rejecting or disambiguating) instead of first-write wins.
Useful? React with 👍 / 👎.
…ns, add collision test
Remaining translators that need the same fixThis PR covers
Each needs the same pattern: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 755ca75879
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| out := make(map[string]string) | ||
| tools.ForEach(func(_, tool gjson.Result) bool { | ||
| name := strings.TrimSpace(tool.Get("name").String()) |
There was a problem hiding this comment.
SanitizedToolNameMap trims each tool name before generating the reverse lookup key, but both updated request translators sanitize the raw (untrimmed) name when emitting functionDeclaration.name/functionCall.name. For names with leading or trailing whitespace (e.g. " foo"), upstream sees "_foo" while this map is computed from "foo" and often omits the entry entirely, so RestoreSanitizedToolName cannot restore the original client tool name and the client may fail to dispatch the returned tool call.
Useful? React with 👍 / 👎.
…slators Apply SanitizeFunctionName on request and RestoreSanitizedToolName on response for: gemini/claude, gemini/openai/chat-completions, gemini/openai/responses, antigravity/openai/chat-completions, gemini-cli/openai/chat-completions. Also update SanitizedToolNameMap to handle OpenAI format (tools[].function.name) in addition to Claude format (tools[].name).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8bb350467
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| name := strings.TrimSpace(tool.Get("name").String()) | ||
| if name == "" { | ||
| return true |
There was a problem hiding this comment.
SanitizedToolNameMap only reads tools[].name and skips entries when that field is empty, but Chat Completions tools carry the name at tools[].function.name (see the request translators iterating t.Get("function") before sanitizing). As a result, the new OpenAI response paths that call RestoreSanitizedToolName (e.g. ConvertGeminiResponseToOpenAI and ConvertAntigravityResponseToOpenAI) get a nil/empty map for OpenAI requests and emit sanitized tool names back to clients; when the original tool was something like mcp/server/read, clients that dispatch by the declared name cannot find a matching tool.
Useful? React with 👍 / 👎.
…-names fix(translator): sanitize tool names for Gemini function_declarations compatibility
2 participants
Summary
Claude Code and MCP clients may send tool definitions with names containing characters invalid for Gemini's
function_declarationsformat (e.g./,@,#, spaces). The proxy currently passes these names through to upstream without sanitization, causing 400 INVALID_ARGUMENT errors from Gemini APIs.This PR sanitizes tool names on request via the existing
SanitizeFunctionNameutility and restores original names on response, applied to bothantigravity/claudeandgemini-cli/claudetranslators.Error Logs
Gemini rejects the request with:
{ "error": { "code": 400, "message": "The GenerateContentRequest proto is invalid:\n * tools[0].function_declarations[0].name: [FIELD_INVALID] Invalid function name. Must start with a letter or an underscore. Must be a-z, A-Z, 0-9, or contain underscores (_), dots (.), colons (:), or dashes (-), with a maximum length of 64.", "status": "INVALID_ARGUMENT" } }Claude models via Antigravity also reject names with
/:{ "error": { "code": 400, "message": "{\"type\":\"error\",\"error\":{\"type\":\"invalid_request_error\",\"message\":\"tools.0.custom.name: String should match pattern '^[a-zA-Z0-9_-]{1,128}$'\"},\"request_id\":\"req_vrtx_...\"}", "status": "INVALID_ARGUMENT" } }Common trigger: Claude Code with MCP tools sending names like
mcp/server/read_file,tool@v2, etc.Changes
internal/util/translator.goSanitizedToolNameMap()andRestoreSanitizedToolName()— build sanitized→original reverse map and lookupinternal/translator/antigravity/claude/antigravity_claude_request.goSanitizeFunctionName()at 4 points: tool declarations,functionCall.name,functionResponse.name,allowedFunctionNamesinternal/translator/antigravity/claude/antigravity_claude_response.goToolNameMapto Params; restore original names in streaming and non-streaming pathsinternal/translator/gemini-cli/claude/gemini-cli_claude_request.gointernal/translator/gemini-cli/claude/gemini-cli_claude_response.gointernal/util/sanitize_test.goSanitizedToolNameMapandRestoreSanitizedToolNameData Flow
Design Notes
SanitizeFunctionNameis a no-op for already-valid names (e.g.Read,Bash,get_weather) — zero overhead for common casesSanitizedToolNameMapreturnsnilwhen no sanitization is needed — response path skips map lookup entirelySanitizeFunctionNameininternal/util/util.gowas already defined but never used in translatorsTesting
SanitizedToolNameMapandRestoreSanitizedToolName