diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_request.go b/internal/translator/openai/openai/responses/openai_openai-responses_request.go index a329d4e3..7ab2ced9 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_request.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_request.go @@ -311,25 +311,8 @@ func ConvertOpenAIResponsesRequestToOpenAIChatCompletions(modelName string, inpu // "additional_tools" input item instead of the top-level "tools" field, // so merge both sources. var chatCompletionsTools []interface{} - appendChatTools := func(tools gjson.Result) { - if !tools.Exists() || !tools.IsArray() { - return - } - tools.ForEach(func(_, tool gjson.Result) bool { - for _, chatTool := range convertResponsesToolToOpenAIChatTools(tool) { - chatCompletionsTools = append(chatCompletionsTools, gjson.ParseBytes(chatTool).Value()) - } - return true - }) - } - appendChatTools(root.Get("tools")) - if input := root.Get("input"); input.Exists() && input.IsArray() { - input.ForEach(func(_, item gjson.Result) bool { - if item.Get("type").String() == "additional_tools" { - appendChatTools(item.Get("tools")) - } - return true - }) + for _, chatTool := range mergeResponsesRequestChatTools(root) { + chatCompletionsTools = append(chatCompletionsTools, gjson.ParseBytes(chatTool).Value()) } if len(chatCompletionsTools) > 0 { out, _ = sjson.SetBytes(out, "tools", chatCompletionsTools) diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go b/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go index 749185a0..48fdcc91 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_request_test.go @@ -695,3 +695,278 @@ func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_NormalizesInputIma }) } } + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_DeduplicatesToolsAcrossAdditionalTools(t *testing.T) { + raw := []byte(`{ + "input": [ + {"role":"user","content":"What time is it?"}, + { + "type":"additional_tools", + "tools":[ + {"type":"function","name":"get_time","description":"copy from additional_tools","parameters":{"type":"object","properties":{"tz":{"type":"string"}}}} + ] + } + ], + "tools": [ + {"type":"function","name":"get_time","description":"authoritative top-level definition","parameters":{"type":"object","properties":{"timezone":{"type":"string"}}}} + ] + }`) + t.Logf("input json:\n%s", prettyJSONForTest(raw)) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-v4-flash", raw, false) + t.Logf("output json:\n%s", prettyJSONForTest(out)) + + if got := gjson.GetBytes(out, "tools.#").Int(); got != 1 { + t.Fatalf("tools count = %d, want 1; output=%s", got, out) + } + if got := gjson.GetBytes(out, "tools.0.function.name").String(); got != "get_time" { + t.Fatalf("tools.0.function.name = %q, want get_time; output=%s", got, out) + } + if got := gjson.GetBytes(out, "tools.0.function.description").String(); got != "authoritative top-level definition" { + t.Fatalf("tools.0.function.description = %q, want the top-level definition to win; output=%s", got, out) + } + if got := gjson.GetBytes(out, "tools.0.function.parameters.properties.timezone.type").String(); got != "string" { + t.Fatalf("tools.0.function.parameters should come from the top-level definition; output=%s", out) + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_DeduplicatesNamespaceQualifiedCollision(t *testing.T) { + raw := []byte(`{ + "input": [ + {"role":"user","content":"Patch the file."} + ], + "tools": [ + {"type":"function","name":"editor__apply_patch","parameters":{"type":"object"}}, + { + "type":"namespace", + "name":"editor", + "tools":[{"type":"function","name":"apply_patch","parameters":{"type":"object"}}] + } + ] + }`) + t.Logf("input json:\n%s", prettyJSONForTest(raw)) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-v4-flash", raw, false) + t.Logf("output json:\n%s", prettyJSONForTest(out)) + + if got := gjson.GetBytes(out, "tools.#").Int(); got != 1 { + t.Fatalf("tools count = %d, want 1; output=%s", got, out) + } + if got := gjson.GetBytes(out, "tools.0.function.name").String(); got != "editor__apply_patch" { + t.Fatalf("tools.0.function.name = %q, want editor__apply_patch; output=%s", got, out) + } +} + +func TestConvertOpenAIResponsesRequestToOpenAIChatCompletions_KeepsDistinctToolsFromBothSources(t *testing.T) { + raw := []byte(`{ + "input": [ + {"role":"user","content":"Do the thing."}, + { + "type":"additional_tools", + "tools":[ + {"type":"function","name":"get_date","parameters":{"type":"object"}}, + {"type":"function","name":"get_time","parameters":{"type":"object"}} + ] + } + ], + "tools": [ + {"type":"function","name":"get_time","parameters":{"type":"object"}}, + {"type":"function","name":"get_weather","parameters":{"type":"object"}} + ] + }`) + t.Logf("input json:\n%s", prettyJSONForTest(raw)) + + out := ConvertOpenAIResponsesRequestToOpenAIChatCompletions("deepseek-v4-flash", raw, false) + t.Logf("output json:\n%s", prettyJSONForTest(out)) + + want := []string{"get_time", "get_weather", "get_date"} + if got := gjson.GetBytes(out, "tools.#").Int(); got != int64(len(want)) { + t.Fatalf("tools count = %d, want %d; output=%s", got, len(want), out) + } + for i, wantName := range want { + got := gjson.GetBytes(out, fmt.Sprintf("tools.%d.function.name", i)).String() + if got != wantName { + t.Fatalf("tools.%d.function.name = %q, want %q; output=%s", i, got, wantName, out) + } + } +} + +func TestResponsesSingleCustomToolName_CountsDeduplicatedTools(t *testing.T) { + raw := []byte(`{ + "input": [ + {"role":"user","content":"Patch the file."}, + { + "type":"additional_tools", + "tools":[{"type":"custom","name":"apply_patch","description":"copy"}] + } + ], + "tools": [ + {"type":"custom","name":"apply_patch","description":"authoritative"} + ] + }`) + + name, ok := responsesSingleCustomToolName(raw) + if !ok { + t.Fatalf("responsesSingleCustomToolName ok = false, want true when the only tool is duplicated across both sources") + } + if name != "apply_patch" { + t.Fatalf("responsesSingleCustomToolName name = %q, want apply_patch", name) + } +} + +func TestSplitResponsesQualifiedFunctionCallFromRequest_FirstDeclarationWins(t *testing.T) { + flatFirst := []byte(`{ + "tools": [ + {"type":"function","name":"editor__apply_patch","parameters":{"type":"object"}}, + {"type":"namespace","name":"editor","tools":[{"type":"function","name":"apply_patch","parameters":{"type":"object"}}]} + ] + }`) + namespaceFirst := []byte(`{ + "tools": [ + {"type":"namespace","name":"editor","tools":[{"type":"function","name":"apply_patch","parameters":{"type":"object"}}]}, + {"type":"function","name":"editor__apply_patch","parameters":{"type":"object"}} + ] + }`) + namespaceOnly := []byte(`{ + "tools": [ + {"type":"namespace","name":"mcp__github","tools":[{"type":"function","name":"get_me","parameters":{"type":"object"}}]} + ] + }`) + + tests := []struct { + name string + raw []byte + qualified string + wantName string + wantNamespace string + }{ + // The flat tool is the one that survives merging, so it must stay flat. + {"flat declared first", flatFirst, "editor__apply_patch", "editor__apply_patch", ""}, + // The namespace child survives here, so the call splits back into it. + {"namespace declared first", namespaceFirst, "editor__apply_patch", "apply_patch", "editor"}, + // No collision: unchanged behaviour. + {"namespace only", namespaceOnly, "mcp__github__get_me", "get_me", "mcp__github"}, + // Unknown name falls through untouched. + {"unknown name", flatFirst, "something_else", "something_else", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotName, gotNamespace := splitResponsesQualifiedFunctionCallFromRequest(tt.raw, tt.qualified) + if gotName != tt.wantName || gotNamespace != tt.wantNamespace { + t.Fatalf("split(%q) = (%q, %q), want (%q, %q)", + tt.qualified, gotName, gotNamespace, tt.wantName, tt.wantNamespace) + } + }) + } +} + +func TestSplitResponsesQualifiedFunctionCallFromRequest_MatchesMergedToolIdentity(t *testing.T) { + // Whatever survives the merge must be what reverse translation reports. + raw := []byte(`{ + "tools": [ + {"type":"function","name":"editor__apply_patch","parameters":{"type":"object"}}, + {"type":"namespace","name":"editor","tools":[{"type":"function","name":"apply_patch","parameters":{"type":"object"}}]} + ] + }`) + + merged := mergeResponsesRequestChatTools(gjson.ParseBytes(raw)) + if len(merged) != 1 { + t.Fatalf("merged tool count = %d, want 1", len(merged)) + } + emitted := gjson.GetBytes(merged[0], "function.name").String() + + name, namespace := splitResponsesQualifiedFunctionCallFromRequest(raw, emitted) + if namespace != "" { + t.Fatalf("emitted tool %q came from a flat declaration, but split reported namespace %q", emitted, namespace) + } + if name != emitted { + t.Fatalf("split(%q) name = %q, want %q", emitted, name, emitted) + } +} + +func TestResponsesCustomToolNames_FollowsMergedDeclaration(t *testing.T) { + // Declarations delivered through the two channels may differ in type: a + // top-level function and an "additional_tools" custom tool can flatten to + // the same Chat Completions name. Only the winner may decide whether the + // tool is freeform, otherwise a plain function call comes back as a + // custom_tool_call with unwrapped arguments. + functionFirst := []byte(`{ + "input": [ + {"type":"additional_tools","tools":[{"type":"custom","name":"exec","description":"copy"}]} + ], + "tools": [ + {"type":"function","name":"exec","parameters":{"type":"object"}} + ] + }`) + customFirst := []byte(`{ + "input": [ + {"type":"additional_tools","tools":[{"type":"function","name":"exec","parameters":{"type":"object"}}]} + ], + "tools": [ + {"type":"custom","name":"exec","description":"authoritative"} + ] + }`) + + tests := []struct { + name string + raw []byte + wantCustom bool + }{ + {name: "function declaration wins", raw: functionFirst, wantCustom: false}, + {name: "custom declaration wins", raw: customFirst, wantCustom: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + merged := mergeResponsesRequestChatTools(gjson.ParseBytes(tt.raw)) + if len(merged) != 1 { + t.Fatalf("merged tool count = %d, want 1", len(merged)) + } + // Freeform tools are the ones converted to the single-string shape. + mergedIsCustom := gjson.GetBytes(merged[0], "function.parameters.properties.input").Exists() + if mergedIsCustom != tt.wantCustom { + t.Fatalf("merged tool custom = %v, want %v", mergedIsCustom, tt.wantCustom) + } + + if _, isCustom := responsesCustomToolNames(tt.raw)["exec"]; isCustom != tt.wantCustom { + t.Fatalf("responsesCustomToolNames classified exec as custom = %v, want %v", isCustom, tt.wantCustom) + } + + name, ok := responsesSingleCustomToolName(tt.raw) + if ok != tt.wantCustom { + t.Fatalf("responsesSingleCustomToolName ok = %v, want %v", ok, tt.wantCustom) + } + if ok && name != "exec" { + t.Fatalf("responsesSingleCustomToolName name = %q, want exec", name) + } + }) + } +} + +func TestResponsesCustomToolNames_OnlyReportsMergedTools(t *testing.T) { + // Nested namespaces are not converted, so their children never reach the + // upstream request and must not be classified as freeform tools either. + raw := []byte(`{ + "tools": [ + {"type":"namespace","name":"outer","tools":[ + {"type":"namespace","name":"inner","tools":[{"type":"custom","name":"buried"}]}, + {"type":"custom","name":"reachable"} + ]} + ] + }`) + + mergedNames := make(map[string]struct{}) + for _, chatTool := range mergeResponsesRequestChatTools(gjson.ParseBytes(raw)) { + mergedNames[gjson.GetBytes(chatTool, "function.name").String()] = struct{}{} + } + if _, ok := mergedNames["outer__reachable"]; !ok { + t.Fatalf("merged tool names = %v, want outer__reachable", mergedNames) + } + + for name := range responsesCustomToolNames(raw) { + if _, ok := mergedNames[name]; !ok { + t.Fatalf("responsesCustomToolNames reported %q, which the merge never emits", name) + } + } +} diff --git a/internal/translator/openai/openai/responses/openai_openai-responses_tools.go b/internal/translator/openai/openai/responses/openai_openai-responses_tools.go index d4a9007b..bfdb4c45 100644 --- a/internal/translator/openai/openai/responses/openai_openai-responses_tools.go +++ b/internal/translator/openai/openai/responses/openai_openai-responses_tools.go @@ -7,23 +7,112 @@ import ( "github.com/tidwall/sjson" ) -func convertResponsesToolToOpenAIChatTools(tool gjson.Result) [][]byte { - toolType := strings.TrimSpace(tool.Get("type").String()) - switch toolType { - case "", "function": - if tJSON, ok := convertResponsesFunctionToolToOpenAIChat(tool, ""); ok { - return [][]byte{tJSON} +// responsesToolDeclaration is one Responses tool declaration paired with the +// Chat Completions function name it produces. Namespace children carry both +// their declared name and the owning namespace, so reverse translation can +// restore the split identity. +type responsesToolDeclaration struct { + tool gjson.Result + chatName string + localName string + namespace string + custom bool +} + +// walkResponsesToolDeclarations visits the tool declarations of a Responses +// request in one canonical order: the top-level "tools" field first, then +// Codex Desktop (Responses Lite) "additional_tools" input items, namespace +// children in declaration order. Declarations that produce no Chat Completions +// tool are skipped. Visiting stops early once visit returns false. +// +// Request conversion, reverse name resolution and freeform tool classification +// all traverse through here, so they cannot disagree about which declaration +// backs a given Chat Completions tool name. +func walkResponsesToolDeclarations(root gjson.Result, visit func(responsesToolDeclaration) bool) { + proceed := true + emit := func(tool gjson.Result, namespaceName string) { + if !proceed { + return + } + var custom bool + switch strings.TrimSpace(tool.Get("type").String()) { + case "", "function": + case "custom": + custom = true + default: + return } - case "namespace": - return convertResponsesNamespaceToolToOpenAIChat(tool) - case "custom": - if tJSON, ok := convertResponsesCustomToolToOpenAIChat(tool, ""); ok { - return [][]byte{tJSON} + localName := responsesToolName(tool) + if localName == "" { + return } - default: - return nil + proceed = visit(responsesToolDeclaration{ + tool: tool, + chatName: qualifyResponsesNamespaceToolName(namespaceName, localName), + localName: localName, + namespace: namespaceName, + custom: custom, + }) + } + scan := func(tools gjson.Result) { + if !proceed || !tools.Exists() || !tools.IsArray() { + return + } + tools.ForEach(func(_, tool gjson.Result) bool { + if strings.TrimSpace(tool.Get("type").String()) == "namespace" { + if children := tool.Get("tools"); children.Exists() && children.IsArray() { + namespaceName := strings.TrimSpace(tool.Get("name").String()) + children.ForEach(func(_, child gjson.Result) bool { + emit(child, namespaceName) + return proceed + }) + } + return proceed + } + emit(tool, "") + return proceed + }) + } + + scan(root.Get("tools")) + if input := root.Get("input"); input.Exists() && input.IsArray() { + input.ForEach(func(_, item gjson.Result) bool { + if item.Get("type").String() == "additional_tools" { + scan(item.Get("tools")) + } + return proceed + }) } - return nil +} + +// mergeResponsesRequestChatTools converts every tool declaration in a Responses +// request into Chat Completions form, merging the top-level "tools" field with +// Codex Desktop (Responses Lite) "additional_tools" input items. +// +// Codex clients may deliver the same tool through both channels, and namespace +// qualification can collapse distinct declarations onto one Chat Completions +// name, so entries are deduplicated by function name. The first occurrence +// wins, which keeps the top-level "tools" definition authoritative over the +// "additional_tools" copy. Chat Completions requires tool names to be unique; +// strict upstreams reject the whole request otherwise. +func mergeResponsesRequestChatTools(root gjson.Result) [][]byte { + var merged [][]byte + seenToolNames := make(map[string]struct{}) + walkResponsesToolDeclarations(root, func(declaration responsesToolDeclaration) bool { + if _, duplicate := seenToolNames[declaration.chatName]; duplicate { + return true + } + convert := convertResponsesFunctionToolToOpenAIChat + if declaration.custom { + convert = convertResponsesCustomToolToOpenAIChat + } + if chatTool, ok := convert(declaration.tool, declaration.chatName); ok { + seenToolNames[declaration.chatName] = struct{}{} + merged = append(merged, chatTool) + } + return true + }) + return merged } // convertResponsesCustomToolToOpenAIChat maps a Responses freeform ("custom") @@ -45,32 +134,6 @@ func convertResponsesCustomToolToOpenAIChat(tool gjson.Result, overrideName stri return chatTool, true } -func convertResponsesNamespaceToolToOpenAIChat(tool gjson.Result) [][]byte { - namespaceName := strings.TrimSpace(tool.Get("name").String()) - children := tool.Get("tools") - if !children.Exists() || !children.IsArray() { - return nil - } - - var out [][]byte - children.ForEach(func(_, child gjson.Result) bool { - childName := responsesToolName(child) - qualifiedName := qualifyResponsesNamespaceToolName(namespaceName, childName) - switch strings.TrimSpace(child.Get("type").String()) { - case "", "function": - if tJSON, ok := convertResponsesFunctionToolToOpenAIChat(child, qualifiedName); ok { - out = append(out, tJSON) - } - case "custom": - if tJSON, ok := convertResponsesCustomToolToOpenAIChat(child, qualifiedName); ok { - out = append(out, tJSON) - } - } - return true - }) - return out -} - func convertResponsesFunctionToolToOpenAIChat(tool gjson.Result, overrideName string) ([]byte, bool) { name := strings.TrimSpace(overrideName) if name == "" { @@ -147,43 +210,28 @@ func responsesToolOutputText(output gjson.Result) string { return "" } -// responsesCustomToolNames collects the names of freeform ("custom") tools -// declared in the original Responses request, both in the top-level "tools" -// field and in Codex Desktop "additional_tools" input items. Namespace child -// names use the qualified Chat Completions form. +// responsesCustomToolNames collects the Chat Completions names of the freeform +// ("custom") tools that survive the merge, so response translation only unwraps +// freeform arguments for calls whose winning declaration really was freeform. +// +// Declaration types may differ across the two delivery channels: a top-level +// function and an "additional_tools" custom tool can flatten to the same name. +// Classification therefore follows the same first-wins rule as the merge — +// a discarded custom declaration must not turn a surviving ordinary function +// into a custom_tool_call. func responsesCustomToolNames(requestRawJSON []byte) map[string]struct{} { names := make(map[string]struct{}) - var collect func(gjson.Result, string) - collect = func(tools gjson.Result, namespaceName string) { - if !tools.Exists() || !tools.IsArray() { - return - } - tools.ForEach(func(_, tool gjson.Result) bool { - switch strings.TrimSpace(tool.Get("type").String()) { - case "custom": - name := responsesToolName(tool) - if namespaceName != "" { - name = qualifyResponsesNamespaceToolName(namespaceName, name) - } - if name != "" { - names[name] = struct{}{} - } - case "namespace": - collect(tool.Get("tools"), strings.TrimSpace(tool.Get("name").String())) - } + seenToolNames := make(map[string]struct{}) + walkResponsesToolDeclarations(gjson.ParseBytes(requestRawJSON), func(declaration responsesToolDeclaration) bool { + if _, duplicate := seenToolNames[declaration.chatName]; duplicate { return true - }) - } - root := gjson.ParseBytes(requestRawJSON) - collect(root.Get("tools"), "") - if input := root.Get("input"); input.Exists() && input.IsArray() { - input.ForEach(func(_, item gjson.Result) bool { - if item.Get("type").String() == "additional_tools" { - collect(item.Get("tools"), "") - } - return true - }) - } + } + seenToolNames[declaration.chatName] = struct{}{} + if declaration.custom { + names[declaration.chatName] = struct{}{} + } + return true + }) return names } @@ -193,27 +241,10 @@ func responsesSingleCustomToolName(requestRawJSON []byte) (string, bool) { return "", false } - toolCount := 0 - collect := func(tools gjson.Result) { - if !tools.Exists() || !tools.IsArray() { - return - } - tools.ForEach(func(_, tool gjson.Result) bool { - toolCount += len(convertResponsesToolToOpenAIChatTools(tool)) - return true - }) - } - - root := gjson.ParseBytes(requestRawJSON) - collect(root.Get("tools")) - if input := root.Get("input"); input.Exists() && input.IsArray() { - input.ForEach(func(_, item gjson.Result) bool { - if item.Get("type").String() == "additional_tools" { - collect(item.Get("tools")) - } - return true - }) - } + // Count the tools actually emitted, which are deduplicated by name, so a + // tool delivered through both "tools" and "additional_tools" still counts + // once and freeform unwrapping stays enabled. + toolCount := len(mergeResponsesRequestChatTools(gjson.ParseBytes(requestRawJSON))) for name := range customToolNames { return name, toolCount == 1 } @@ -247,60 +278,35 @@ func qualifyResponsesNamespaceToolName(namespaceName, childName string) string { return namespaceName + "__" + childName } +// resolveResponsesQualifiedToolIdentity maps an emitted Chat Completions +// function name back to the Responses declaration that produced it. +// +// Declarations are walked in the same order mergeResponsesRequestChatTools +// uses, and the first one producing the name wins, so reverse translation +// reports the identity of the declaration that actually survived the merge. A +// flat top-level tool named "editor__apply_patch" therefore stays flat even +// when a later namespace declares a child qualifying to the same name. +func resolveResponsesQualifiedToolIdentity(root gjson.Result, qualifiedName string) (name, namespace string, found bool) { + walkResponsesToolDeclarations(root, func(declaration responsesToolDeclaration) bool { + if declaration.chatName != qualifiedName { + return true + } + name, namespace, found = declaration.localName, declaration.namespace, true + return false + }) + return name, namespace, found +} + func splitResponsesQualifiedFunctionCallFromRequest(requestRawJSON []byte, qualifiedName string) (name, namespace string) { qualifiedName = strings.TrimSpace(qualifiedName) if qualifiedName == "" { return "", "" } - var bestNamespace string - var bestChild string - collect := func(tools gjson.Result) { - if !tools.Exists() || !tools.IsArray() { - return - } - tools.ForEach(func(_, tool gjson.Result) bool { - if strings.TrimSpace(tool.Get("type").String()) != "namespace" { - return true - } - namespaceName := strings.TrimSpace(tool.Get("name").String()) - if namespaceName == "" { - return true - } - children := tool.Get("tools") - if !children.Exists() || !children.IsArray() { - return true - } - children.ForEach(func(_, child gjson.Result) bool { - childName := responsesToolName(child) - if childName == "" { - return true - } - if qualifyResponsesNamespaceToolName(namespaceName, childName) == qualifiedName { - bestNamespace = namespaceName - bestChild = childName - } - return true - }) - return true - }) - } - - root := gjson.ParseBytes(requestRawJSON) - collect(root.Get("tools")) - if input := root.Get("input"); input.Exists() && input.IsArray() { - input.ForEach(func(_, item gjson.Result) bool { - if item.Get("type").String() == "additional_tools" { - collect(item.Get("tools")) - } - return true - }) - } - - if bestNamespace == "" || bestChild == "" { - return qualifiedName, "" + if resolvedName, resolvedNamespace, ok := resolveResponsesQualifiedToolIdentity(gjson.ParseBytes(requestRawJSON), qualifiedName); ok { + return resolvedName, resolvedNamespace } - return bestChild, bestNamespace + return qualifiedName, "" } func pickRequestJSON(originalRequestRawJSON, requestRawJSON []byte) []byte {