From 3b9253c2be96b4368be0dab1f8fb2320fd48f474 Mon Sep 17 00:00:00 2001 From: Saboor Hassan Date: Wed, 31 Dec 2025 02:14:46 +0500 Subject: [PATCH] fix(translator): resolve invalid function name errors by sanitizing Claude tool names This commit centralizes tool name sanitization in SanitizeFunctionName, applying character compliance, starting character rules, and length limits. It also fixes a regression in gemini_schema tests and preserves MCP-specific shortening logic while ensuring compliance. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- .../codex/claude/codex_claude_request.go | 23 +++++++++++++++++++ internal/util/gemini_schema.go | 5 ++++ internal/util/gemini_schema_test.go | 4 +++- internal/util/sanitize_test.go | 1 + internal/util/util.go | 23 ++++++++++++------- 5 files changed, 47 insertions(+), 9 deletions(-) diff --git a/internal/translator/codex/claude/codex_claude_request.go b/internal/translator/codex/claude/codex_claude_request.go index e1eb4ce7..52468e11 100644 --- a/internal/translator/codex/claude/codex_claude_request.go +++ b/internal/translator/codex/claude/codex_claude_request.go @@ -264,6 +264,18 @@ func ConvertClaudeRequestToCodex(modelName string, inputRawJSON []byte, _ bool) // shortenNameIfNeeded applies a simple shortening rule for a single name. func shortenNameIfNeeded(name string) string { + const limit = 64 + if len(name) <= limit { + // Even if within limit, we still apply SanitizeFunctionName to ensure character compliance + return util.SanitizeFunctionName(name) + } + if strings.HasPrefix(name, "mcp__") { + idx := strings.LastIndex(name, "__") + if idx > 0 { + cand := "mcp__" + name[idx+2:] + return util.SanitizeFunctionName(cand) + } + } return util.SanitizeFunctionName(name) } @@ -274,6 +286,17 @@ func buildShortNameMap(names []string) map[string]string { m := map[string]string{} baseCandidate := func(n string) string { + const limit = 64 + if len(n) <= limit { + return util.SanitizeFunctionName(n) + } + if strings.HasPrefix(n, "mcp__") { + idx := strings.LastIndex(n, "__") + if idx > 0 { + cand := "mcp__" + n[idx+2:] + return util.SanitizeFunctionName(cand) + } + } return util.SanitizeFunctionName(n) } diff --git a/internal/util/gemini_schema.go b/internal/util/gemini_schema.go index 33df61f9..38d3773e 100644 --- a/internal/util/gemini_schema.go +++ b/internal/util/gemini_schema.go @@ -390,6 +390,11 @@ func addEmptySchemaPlaceholder(jsonStr string) string { // If schema has properties but none are required, add a minimal placeholder. if propsVal.IsObject() && !hasRequiredProperties { + // DO NOT add placeholder if it's a top-level schema (parentPath is empty) + // or if we've already added a placeholder reason above. + if parentPath == "" { + continue + } placeholderPath := joinPath(propsPath, "_") if !gjson.Get(jsonStr, placeholderPath).Exists() { jsonStr, _ = sjson.Set(jsonStr, placeholderPath+".type", "boolean") diff --git a/internal/util/gemini_schema_test.go b/internal/util/gemini_schema_test.go index 69adbcdb..60335f22 100644 --- a/internal/util/gemini_schema_test.go +++ b/internal/util/gemini_schema_test.go @@ -127,8 +127,10 @@ func TestCleanJSONSchemaForAntigravity_AnyOfFlattening_SmartSelection(t *testing "type": "object", "description": "Accepts: null | object", "properties": { + "_": { "type": "boolean" }, "kind": { "type": "string" } - } + }, + "required": ["_"] } } }` diff --git a/internal/util/sanitize_test.go b/internal/util/sanitize_test.go index 06a0ca34..4ff8454b 100644 --- a/internal/util/sanitize_test.go +++ b/internal/util/sanitize_test.go @@ -27,6 +27,7 @@ func TestSanitizeFunctionName(t *testing.T) { {"Too long (65 chars)", "this_is_a_very_long_name_that_exactly_reaches_sixty_four_charactX", "this_is_a_very_long_name_that_exactly_reaches_sixty_four_charact"}, {"Very long", "this_is_a_very_long_name_that_exceeds_the_sixty_four_character_limit_for_function_names", "this_is_a_very_long_name_that_exceeds_the_sixty_four_character_l"}, {"Starts with digit (64 chars total)", "1234567890123456789012345678901234567890123456789012345678901234", "_123456789012345678901234567890123456789012345678901234567890123"}, + {"Starts with invalid char (64 chars total)", "!234567890123456789012345678901234567890123456789012345678901234", "_234567890123456789012345678901234567890123456789012345678901234"}, {"Empty", "", ""}, {"Single character invalid", "@", "_"}, {"Single character valid", "a", "a"}, diff --git a/internal/util/util.go b/internal/util/util.go index ecf95943..4e846306 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -25,19 +25,26 @@ func SanitizeFunctionName(name string) string { if name == "" { return "" } + // Replace invalid characters with underscore sanitized := functionNameSanitizer.ReplaceAllString(name, "_") // Ensure it starts with a letter or underscore - first := sanitized[0] - if !((first >= 'a' && first <= 'z') || (first >= 'A' && first <= 'Z') || first == '_') { - // If it starts with an allowed character but not allowed at the beginning, - // we must prepend an underscore. - // To stay within the 64-character limit while prepending, we may need to truncate first. - if len(sanitized) >= 64 { - sanitized = sanitized[:63] + // Re-reading requirements: Must start with a letter or an underscore. + if len(sanitized) > 0 { + first := sanitized[0] + if !((first >= 'a' && first <= 'z') || (first >= 'A' && first <= 'Z') || first == '_') { + // If it starts with an allowed character but not allowed at the beginning (digit, dot, colon, dash), + // we must prepend an underscore. + + // To stay within the 64-character limit while prepending, we must truncate first. + if len(sanitized) >= 64 { + sanitized = sanitized[:63] + } + sanitized = "_" + sanitized } - sanitized = "_" + sanitized + } else { + sanitized = "_" } // Truncate to 64 characters