diff --git a/backend/internal/pkg/apicompat/responses_client_tools.go b/backend/internal/pkg/apicompat/responses_client_tools.go index b7ce024e8a..b8c2c99cbd 100644 --- a/backend/internal/pkg/apicompat/responses_client_tools.go +++ b/backend/internal/pkg/apicompat/responses_client_tools.go @@ -212,12 +212,12 @@ func rewriteClientToolHistory(value any, adapter *ResponsesClientToolMapping) (b typed["type"] = "function_call" typed["arguments"] = customToolCallArguments(stringValue(typed["input"])) delete(typed, "input") - dropInvalidLoweredFunctionItemID(typed) + normalizeLoweredFunctionItemID(typed) changed = true } case "custom_tool_call_output": typed["type"] = "function_call_output" - dropInvalidLoweredFunctionItemID(typed) + normalizeLoweredFunctionItemID(typed) normalizeClientToolOutput(typed) changed = true case "tool_search_call": @@ -226,7 +226,7 @@ func rewriteClientToolHistory(value any, adapter *ResponsesClientToolMapping) (b typed["name"] = toolSearchProxyName typed["arguments"] = rawObjectString(typed["arguments"]) delete(typed, "execution") - dropInvalidLoweredFunctionItemID(typed) + normalizeLoweredFunctionItemID(typed) changed = true } case "tool_search_output": @@ -236,7 +236,7 @@ func rewriteClientToolHistory(value any, adapter *ResponsesClientToolMapping) (b return fmt.Errorf("tool_search_output requires a non-empty string call_id before it can be lowered to function_call_output") } typed["type"] = "function_call_output" - dropInvalidLoweredFunctionItemID(typed) + normalizeLoweredFunctionItemID(typed) if err := normalizeToolSearchOutput(typed); err != nil { return err } @@ -257,14 +257,73 @@ func rewriteClientToolHistory(value any, adapter *ResponsesClientToolMapping) (b return changed, nil } -// dropInvalidLoweredFunctionItemID removes Codex client-only item IDs such as +// normalizeLoweredFunctionItemID reconciles Codex client-only item IDs such as // ctc_*, ctco_*, tsc_*, and tso_* after their item type is lowered to the -// function protocol. Function upstreams validate these IDs with the fc prefix; -// call_id, which is preserved separately, is the tool call/output pairing key. -func dropInvalidLoweredFunctionItemID(item map[string]any) { +// function protocol, which validates these IDs with the fc prefix. A ctc_/tsc_ +// call ID maps back to the fc_ ID it was raised from; the remaining IDs have no +// function-protocol counterpart and are dropped. call_id, which is preserved +// separately, stays the tool call/output pairing key either way. +func normalizeLoweredFunctionItemID(item map[string]any) { id := strings.TrimSpace(stringValue(item["id"])) - if id != "" && !strings.HasPrefix(id, "fc") { - delete(item, "id") + if id == "" || strings.HasPrefix(id, "fc") { + return + } + // A ctc_/tsc_ call ID on a lowered call item is the one we minted from the + // upstream's own fc_ ID when the item was raised, so map it back instead of + // dropping it. Anything else has no known function-protocol counterpart. + if recovered := retypedResponsesToolCallItemID(id, "function_call"); recovered != id { + item["id"] = recovered + return + } + delete(item, "id") +} + +// responsesToolCallItemIDPrefixes lists the Responses item ID prefixes that are +// tied to a specific tool-call item type. +var responsesToolCallItemIDPrefixes = []string{"fc_", "ctc_", "tsc_"} + +// responsesToolCallItemIDPrefix reports the ID prefix the Responses API +// validates for itemType, or "" when the type constrains no prefix. +func responsesToolCallItemIDPrefix(itemType string) string { + switch itemType { + case "custom_tool_call": + return "ctc_" + case "tool_search_call": + return "tsc_" + case "function_call": + return "fc_" + default: + return "" + } +} + +// retypedResponsesToolCallItemID re-prefixes an upstream item ID so it agrees +// with the item type we raise it back to. A function-only upstream answers a +// lowered custom tool with an fc_ item ID; emitting that ID on the restored +// custom_tool_call poisons the client's history, because a later replay of the +// same item to an upstream that validates IDs fails with +// "Invalid 'input[N].id' ... Expected an ID that begins with 'ctc'". +// The suffix is preserved so the ID stays stable and unique per upstream item. +// IDs that carry no known tool-call prefix are left alone rather than guessed at. +func retypedResponsesToolCallItemID(id, itemType string) string { + want := responsesToolCallItemIDPrefix(itemType) + if want == "" || id == "" || strings.HasPrefix(id, want) { + return id + } + for _, known := range responsesToolCallItemIDPrefixes { + if known != want && strings.HasPrefix(id, known) { + return want + strings.TrimPrefix(id, known) + } + } + return id +} + +// retypeResponsesToolCallItemID applies retypedResponsesToolCallItemID to a +// decoded item map. +func retypeResponsesToolCallItemID(item map[string]any, itemType string) { + id := strings.TrimSpace(stringValue(item["id"])) + if retyped := retypedResponsesToolCallItemID(id, itemType); retyped != id { + item["id"] = retyped } } @@ -409,12 +468,14 @@ func restoreClientToolValue(value any, adapter *ResponsesClientToolMapping) bool name := strings.TrimSpace(stringValue(typed["name"])) if adapter.CustomTools[name] { typed["type"] = "custom_tool_call" + retypeResponsesToolCallItemID(typed, "custom_tool_call") typed["input"] = extractCustomToolCallInput(rawObjectString(typed["arguments"])) delete(typed, "arguments") delete(typed, "namespace") changed = true } else if adapter.ToolSearch && name == toolSearchProxyName { typed["type"] = "tool_search_call" + retypeResponsesToolCallItemID(typed, "tool_search_call") typed["execution"] = "client" typed["arguments"] = json.RawMessage(toolSearchCallArgumentsJSON(rawObjectString(typed["arguments"]))) delete(typed, "name") @@ -441,12 +502,15 @@ type ResponsesClientToolStreamRestorer struct { } type responsesClientToolStreamCall struct { - kind string - name string - callID string - itemID string - outputIdx int - arguments strings.Builder + kind string + name string + // callID and itemID stay as the upstream sent them so later upstream + // events keep matching this call; clientItemID is what we emit. + callID string + itemID string + clientItemID string + outputIdx int + arguments strings.Builder } func NewResponsesClientToolStreamRestorer(mapping ResponsesClientToolMapping) *ResponsesClientToolStreamRestorer { @@ -485,6 +549,9 @@ func (r *ResponsesClientToolStreamRestorer) Restore(event ResponsesStreamEvent) event.Item.Arguments = "{}" event.Item.Namespace = "" } + if call.clientItemID != "" { + event.Item.ID = call.clientItemID + } } emit(r.restoreNamespaceEvent(event)) case "response.function_call_arguments.delta": @@ -502,9 +569,9 @@ func (r *ResponsesClientToolStreamRestorer) Restore(event ResponsesStreamEvent) if call.kind == "custom" { input := extractCustomToolCallInput(call.arguments.String()) if input != "" { - emit(ResponsesStreamEvent{Type: "response.custom_tool_call_input.delta", OutputIndex: call.outputIdx, ItemID: call.itemID, Delta: input}) + emit(ResponsesStreamEvent{Type: "response.custom_tool_call_input.delta", OutputIndex: call.outputIdx, ItemID: call.clientItemID, Delta: input}) } - emit(ResponsesStreamEvent{Type: "response.custom_tool_call_input.done", OutputIndex: call.outputIdx, ItemID: call.itemID, CallID: call.callID, Name: call.name, Input: input}) + emit(ResponsesStreamEvent{Type: "response.custom_tool_call_input.done", OutputIndex: call.outputIdx, ItemID: call.clientItemID, CallID: call.callID, Name: call.name, Input: input}) } return out } @@ -525,6 +592,9 @@ func (r *ResponsesClientToolStreamRestorer) Restore(event ResponsesStreamEvent) } event.Item.Namespace = "" } + if call.clientItemID != "" { + event.Item.ID = call.clientItemID + } delete(r.calls, call.itemID) delete(r.calls, call.callID) delete(r.byOutput, call.outputIdx) @@ -661,6 +731,15 @@ func (r *ResponsesClientToolStreamRestorer) resequenceRaw(payload []byte, sequen return [][]byte{encoded}, true, nil } +// responsesClientToolItemType maps a restorer call kind to the item type the +// client sees. +func responsesClientToolItemType(kind string) string { + if kind == "custom" { + return "custom_tool_call" + } + return "tool_search_call" +} + func (r *ResponsesClientToolStreamRestorer) recordItem(event ResponsesStreamEvent) *responsesClientToolStreamCall { if event.Item == nil || event.Item.Type != "function_call" { return nil @@ -681,7 +760,14 @@ func (r *ResponsesClientToolStreamRestorer) recordItem(event ResponsesStreamEven } call := r.calls[key] if call == nil { - call = &responsesClientToolStreamCall{kind: kind, name: name, callID: event.Item.CallID, itemID: event.Item.ID, outputIdx: event.OutputIndex} + call = &responsesClientToolStreamCall{ + kind: kind, + name: name, + callID: event.Item.CallID, + itemID: event.Item.ID, + clientItemID: retypedResponsesToolCallItemID(event.Item.ID, responsesClientToolItemType(kind)), + outputIdx: event.OutputIndex, + } r.calls[key] = call if call.callID != "" { r.calls[call.callID] = call @@ -735,11 +821,13 @@ func restoreResponsesOutputClientTools(outputs []ResponsesOutput, adapter *Respo } if adapter.CustomTools[output.Name] { output.Type = "custom_tool_call" + output.ID = retypedResponsesToolCallItemID(output.ID, output.Type) output.Input = extractCustomToolCallInput(output.Arguments) output.Arguments = "" output.Namespace = "" } else if adapter.ToolSearch && output.Name == toolSearchProxyName { output.Type = "tool_search_call" + output.ID = retypedResponsesToolCallItemID(output.ID, output.Type) output.Name = "" output.Namespace = "" } diff --git a/backend/internal/pkg/apicompat/responses_client_tools_item_id_helper_test.go b/backend/internal/pkg/apicompat/responses_client_tools_item_id_helper_test.go new file mode 100644 index 0000000000..66448ed661 --- /dev/null +++ b/backend/internal/pkg/apicompat/responses_client_tools_item_id_helper_test.go @@ -0,0 +1,29 @@ +package apicompat + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRetypedResponsesToolCallItemID(t *testing.T) { + for _, tc := range []struct { + name string + id string + itemType string + want string + }{ + {"function id raised to custom", "fc_abc", "custom_tool_call", "ctc_abc"}, + {"function id raised to tool search", "fc_abc", "tool_search_call", "tsc_abc"}, + {"already correct is untouched", "ctc_abc", "custom_tool_call", "ctc_abc"}, + {"custom id lowered to function", "ctc_abc", "function_call", "fc_abc"}, + {"unknown prefix is left alone", "item_abc", "custom_tool_call", "item_abc"}, + {"unprefixed id is left alone", "abc", "custom_tool_call", "abc"}, + {"empty id stays empty", "", "custom_tool_call", ""}, + {"unconstrained item type is left alone", "fc_abc", "message", "fc_abc"}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, retypedResponsesToolCallItemID(tc.id, tc.itemType)) + }) + } +} diff --git a/backend/internal/pkg/apicompat/responses_client_tools_item_id_test.go b/backend/internal/pkg/apicompat/responses_client_tools_item_id_test.go new file mode 100644 index 0000000000..724f77b75d --- /dev/null +++ b/backend/internal/pkg/apicompat/responses_client_tools_item_id_test.go @@ -0,0 +1,132 @@ +package apicompat + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// A function-only upstream answers a lowered custom tool with an fc_ item ID. +// Emitting that ID on the restored custom_tool_call breaks the client the next +// time the item is replayed to an upstream that validates item IDs: +// "Invalid 'input[N].id': 'fc_...'. Expected an ID that begins with 'ctc'". +func TestRestoreResponsesClientToolPayload_RetypesToolCallItemIDs(t *testing.T) { + mapping := ResponsesClientToolMapping{ + CustomTools: map[string]bool{"exec": true}, ToolSearch: true, + NamespaceTools: map[string]ResponsesNamespaceName{"team__send": {Namespace: "team", Name: "send"}}, + } + payload := []byte(`{"id":"resp","output":[` + + `{"type":"function_call","id":"fc_abc123","call_id":"call_1","name":"exec","arguments":"{\"input\":\"dir\"}"},` + + `{"type":"function_call","id":"fc_def456","call_id":"call_2","name":"tool_search","arguments":"{\"query\":\"git\"}"},` + + `{"type":"function_call","id":"fc_ghi789","call_id":"call_3","name":"team__send","arguments":"{}"}]}`) + + restored, changed, err := RestoreResponsesClientToolPayload(payload, mapping) + require.NoError(t, err) + require.True(t, changed) + require.JSONEq(t, `{"id":"resp","output":[`+ + `{"type":"custom_tool_call","id":"ctc_abc123","call_id":"call_1","name":"exec","input":"dir"},`+ + `{"type":"tool_search_call","id":"tsc_def456","call_id":"call_2","execution":"client","arguments":{"query":"git"}},`+ + `{"type":"function_call","id":"fc_ghi789","call_id":"call_3","name":"send","namespace":"team","arguments":"{}"}]}`, + string(restored)) +} + +func TestRestoreResponsesOutputClientTools_RetypesToolCallItemIDs(t *testing.T) { + mapping := ResponsesClientToolMapping{CustomTools: map[string]bool{"exec": true}, ToolSearch: true} + outputs := []ResponsesOutput{ + {Type: "function_call", ID: "fc_abc123", CallID: "call_1", Name: "exec", Arguments: `{"input":"dir"}`}, + {Type: "function_call", ID: "fc_def456", CallID: "call_2", Name: toolSearchProxyName, Arguments: `{"query":"git"}`}, + } + + restoreResponsesOutputClientTools(outputs, &mapping) + + require.Equal(t, "custom_tool_call", outputs[0].Type) + require.Equal(t, "ctc_abc123", outputs[0].ID) + require.Equal(t, "call_1", outputs[0].CallID) + require.Equal(t, "tool_search_call", outputs[1].Type) + require.Equal(t, "tsc_def456", outputs[1].ID) + require.Equal(t, "call_2", outputs[1].CallID) +} + +func TestResponsesClientToolStreamRestorer_RetypesCustomToolCallItemID(t *testing.T) { + const upstreamID = "fc_09f77ac43cf7db36016a8920e7934487" + const clientID = "ctc_09f77ac43cf7db36016a8920e7934487" + + restorer := NewResponsesClientToolStreamRestorer(ResponsesClientToolMapping{CustomTools: map[string]bool{"exec": true}}) + + added := restorer.Restore(ResponsesStreamEvent{ + Type: "response.output_item.added", SequenceNumber: 0, OutputIndex: 0, + Item: &ResponsesOutput{Type: "function_call", ID: upstreamID, CallID: "call_1", Name: "exec", Status: "in_progress"}, + }) + require.Len(t, added, 1) + require.Equal(t, "custom_tool_call", added[0].Item.Type) + require.Equal(t, clientID, added[0].Item.ID) + + // Later upstream events still address the item by its upstream ID. + require.Empty(t, restorer.Restore(ResponsesStreamEvent{ + Type: "response.function_call_arguments.delta", SequenceNumber: 1, ItemID: upstreamID, Delta: `{"input":"di`, + })) + done := restorer.Restore(ResponsesStreamEvent{ + Type: "response.function_call_arguments.done", SequenceNumber: 2, ItemID: upstreamID, + CallID: "call_1", Name: "exec", Arguments: `{"input":"dir"}`, + }) + require.Len(t, done, 2) + require.Equal(t, "response.custom_tool_call_input.delta", done[0].Type) + require.Equal(t, clientID, done[0].ItemID) + require.Equal(t, "response.custom_tool_call_input.done", done[1].Type) + require.Equal(t, clientID, done[1].ItemID) + require.Equal(t, "call_1", done[1].CallID) + + closed := restorer.Restore(ResponsesStreamEvent{ + Type: "response.output_item.done", SequenceNumber: 3, OutputIndex: 0, + Item: &ResponsesOutput{Type: "function_call", ID: upstreamID, CallID: "call_1", Name: "exec", Arguments: `{"input":"dir"}`, Status: "completed"}, + }) + require.Len(t, closed, 1) + require.Equal(t, "custom_tool_call", closed[0].Item.Type) + require.Equal(t, clientID, closed[0].Item.ID) + require.Equal(t, "dir", closed[0].Item.Input) +} + +func TestResponsesClientToolStreamRestorer_RetypesToolSearchCallItemID(t *testing.T) { + restorer := NewResponsesClientToolStreamRestorer(ResponsesClientToolMapping{ToolSearch: true}) + + added := restorer.Restore(ResponsesStreamEvent{ + Type: "response.output_item.added", SequenceNumber: 0, OutputIndex: 0, + Item: &ResponsesOutput{Type: "function_call", ID: "fc_search1", CallID: "call_1", Name: toolSearchProxyName, Status: "in_progress"}, + }) + require.Len(t, added, 1) + require.Equal(t, "tool_search_call", added[0].Item.Type) + require.Equal(t, "tsc_search1", added[0].Item.ID) +} + +// The WS bridge replays restored items back to the upstream, so the ID we hand +// the client has to map back to the upstream's own fc_ ID on the way down. +func TestAdaptResponsesClientTools_RecoversRetypedToolCallItemID(t *testing.T) { + req := map[string]any{ + "tools": []any{ + map[string]any{"type": "custom", "name": "exec"}, + map[string]any{"type": "tool_search"}, + }, + "input": []any{ + map[string]any{"type": "custom_tool_call", "id": "ctc_upstream1", "call_id": "call_1", "name": "exec", "input": "dir"}, + map[string]any{"type": "tool_search_call", "id": "tsc_upstream2", "call_id": "call_2", "arguments": map[string]any{"query": "git"}}, + map[string]any{"type": "custom_tool_call_output", "id": "ctco_client", "call_id": "call_1", "output": "ok"}, + }, + } + + _, changed, err := AdaptResponsesClientTools(req) + require.NoError(t, err) + require.True(t, changed) + + input := requireResponsesClientToolValue[[]any](t, req["input"]) + require.Len(t, input, 3) + customCall := requireResponsesClientToolValue[map[string]any](t, input[0]) + require.Equal(t, "function_call", customCall["type"]) + require.Equal(t, "fc_upstream1", customCall["id"]) + searchCall := requireResponsesClientToolValue[map[string]any](t, input[1]) + require.Equal(t, "function_call", searchCall["type"]) + require.Equal(t, "fc_upstream2", searchCall["id"]) + // Output items have no function-protocol ID counterpart and stay dropped. + customOutput := requireResponsesClientToolValue[map[string]any](t, input[2]) + require.Equal(t, "function_call_output", customOutput["type"]) + require.NotContains(t, customOutput, "id") +} diff --git a/backend/internal/pkg/apicompat/responses_client_tools_test.go b/backend/internal/pkg/apicompat/responses_client_tools_test.go index 5b0dac4356..21d96e54ac 100644 --- a/backend/internal/pkg/apicompat/responses_client_tools_test.go +++ b/backend/internal/pkg/apicompat/responses_client_tools_test.go @@ -48,14 +48,14 @@ func TestAdaptResponsesClientTools_LowersDeclarationsHistoryChoiceAndNamespaces( input := requireResponsesClientToolValue[[]any](t, req["input"]) customCall := requireResponsesClientToolValue[map[string]any](t, input[0]) require.Equal(t, "function_call", customCall["type"]) - require.NotContains(t, customCall, "id") + require.Equal(t, "fc_client", customCall["id"]) require.JSONEq(t, `{"input":"dir"}`, requireResponsesClientToolValue[string](t, customCall["arguments"])) customOutput := requireResponsesClientToolValue[map[string]any](t, input[1]) require.Equal(t, "function_call_output", customOutput["type"]) require.NotContains(t, customOutput, "id") searchCall := requireResponsesClientToolValue[map[string]any](t, input[2]) require.Equal(t, "function_call", searchCall["type"]) - require.NotContains(t, searchCall, "id") + require.Equal(t, "fc_client", searchCall["id"]) require.Equal(t, toolSearchProxyName, searchCall["name"]) require.JSONEq(t, `{"query":"git"}`, requireResponsesClientToolValue[string](t, searchCall["arguments"])) searchOutput := requireResponsesClientToolValue[map[string]any](t, input[3])