From 3c67b5ca5eabe15847bd2e0426da247bd158bf97 Mon Sep 17 00:00:00 2001 From: volcanic <72688335+volcanicll@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:14:58 +0800 Subject: [PATCH] =?UTF-8?q?gateway:=20a=20repeated=20tool=20call=20id=20co?= =?UTF-8?q?ntinues=20the=20call=20it=20names,=20not=20a=20new=20call=20?= =?UTF-8?q?=E2=80=94=20some=20relays=20repeat=20a=20tool=20call's=20id=20o?= =?UTF-8?q?n=20every=20fragment=20of=20a=20streamed=20reply,=20where=20the?= =?UTF-8?q?=20OpenAI=20spec=20sends=20it=20on=20the=20first=20alone.=20The?= =?UTF-8?q?=20decoder=20read=20any=20non-empty=20id=20as=20"a=20new=20call?= =?UTF-8?q?=20starts",=20so=20one=20tool=20call=20came=20out=20of=20transl?= =?UTF-8?q?ation=20as=20one=20call=20per=20fragment:=20chatEncoder=20numbe?= =?UTF-8?q?red=20each=20of=20them=20a=20call=20of=20its=20own,=20and=20ant?= =?UTF-8?q?hropicEncoder=20and=20responsesEncoder=20opened=20a=20content?= =?UTF-8?q?=20block=20and=20an=20output=20item=20for=20each,=20every=20one?= =?UTF-8?q?=20holding=20a=20slice=20of=20the=20arguments=20and=20none=20of?= =?UTF-8?q?=20them=20valid=20JSON.=20An=20agent=20asking=20for=20a=20tool?= =?UTF-8?q?=20through=20such=20a=20relay=20was=20answered=20several=20brok?= =?UTF-8?q?en=20calls=20instead=20of=20the=20one=20it=20made=20=E2=80=94?= =?UTF-8?q?=20Claude=20Code,=20which=20asks=20on=20the=20Anthropic=20API,?= =?UTF-8?q?=20among=20them.=20A=20fragment=20now=20continues=20the=20call?= =?UTF-8?q?=20already=20open=20when=20its=20index=20matches=20and=20it=20e?= =?UTF-8?q?ither=20names=20no=20id=20or=20names=20the=20id=20already=20ope?= =?UTF-8?q?n;=20only=20a=20new=20id=20or=20a=20new=20index=20starts=20the?= =?UTF-8?q?=20next=20one.=20(#395)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestChatTranslationToolCallIDRepeated, TestRepeatedToolCallIDOpensOneBlock and TestRepeatedToolCallIDKeepsParallelCalls fail without the change. gofmt clean; go vet ./... passes; go test ./internal/gateway/ and go test -race ./internal/gateway/ ./internal/redact/ pass. go test ./... passes but for internal/agent (TestDriftUnwiredEveryAgent, TestUnsetAgentsReadEmpty), internal/plugin (TestPluginProxy) and internal/profile (TestProfileCarriesLibrary), which fail the same way on main before the change. --- internal/gateway/chat.go | 14 ++- internal/gateway/translate_stream_test.go | 135 ++++++++++++++++++++++ 2 files changed, 144 insertions(+), 5 deletions(-) diff --git a/internal/gateway/chat.go b/internal/gateway/chat.go index e95ce8d0..57639fb0 100644 --- a/internal/gateway/chat.go +++ b/internal/gateway/chat.go @@ -515,6 +515,7 @@ func (u Usage) chat() map[string]any { type chatDecoder struct { started bool tool int // index of the open tool call, -1 for none + toolID string // id of the open tool call, as some relays repeat it on every fragment choice string // index of the first choice seen; an empty string means none yet // Gemini's OpenAI-compatible API, asked for thoughts, may give them // in the text as a leading …: lead holds the text @@ -650,11 +651,14 @@ func (d *chatDecoder) decode(data string, emit func(Event)) error { if tc.Index != nil { idx = *tc.Index } - if tc.ID != "" || tc.Function.Name != "" || idx != d.tool { - if idx != d.tool || tc.ID != "" { - d.tool = idx - emit(Event{Kind: KToolStart, ID: tc.ID, Name: tc.Function.Name}) - } + // A delta carrying the id the open call already goes by — some + // relays repeat it on every fragment, where the spec sends it + // only on the first — or one more fragment of the open index, + // continues that call; only a new id or a new index starts the + // next one. + if idx != d.tool || (tc.ID != "" && tc.ID != d.toolID) { + d.tool, d.toolID = idx, tc.ID + emit(Event{Kind: KToolStart, ID: tc.ID, Name: tc.Function.Name}) } if tc.Function.Arguments != "" { emit(Event{Kind: KToolArgs, Text: tc.Function.Arguments}) diff --git a/internal/gateway/translate_stream_test.go b/internal/gateway/translate_stream_test.go index 26288978..3cdeff3e 100644 --- a/internal/gateway/translate_stream_test.go +++ b/internal/gateway/translate_stream_test.go @@ -251,3 +251,138 @@ func TestStreamedCallStopsForTool(t *testing.T) { t.Fatalf("stop %q", stop) } } + +// A relay can repeat a tool call's id on every fragment of it, where the +// spec sends the id only on the first. Each such fragment must continue +// the call already open: only a new id, a new name or a new index starts +// the next one. Read as a fresh call every time, one tool call came out as +// several, each holding a piece of the arguments. +func TestChatTranslationToolCallIDRepeated(t *testing.T) { + var d chatDecoder + var got []Event + for _, chunk := range []string{ + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","type":"function","function":{"name":"get_weather","arguments":""}}]}}]}`, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"arguments":"{\"ci"}}]}}]}`, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"arguments":"ty\":\"SF\"}"}}]}}]}`, + `{"id":"x","choices":[{"delta":{},"finish_reason":"tool_calls"}]}`, + } { + if err := d.decode(chunk, func(ev Event) { got = append(got, ev) }); err != nil { + t.Fatal(err) + } + } + var starts []Event + var args, stop string + for _, ev := range got { + switch ev.Kind { + case KToolStart: + starts = append(starts, ev) + case KToolArgs: + args += ev.Text + case KStop: + stop = ev.Stop + } + } + if len(starts) != 1 { + t.Fatalf("a repeated id started %d calls, want 1: %+v", len(starts), starts) + } + if starts[0].ID != "call_1" || starts[0].Name != "get_weather" { + t.Fatalf("call = %q %q", starts[0].ID, starts[0].Name) + } + if args != `{"city":"SF"}` { + t.Fatalf("arguments = %q", args) + } + if stop != "tool" { + t.Fatalf("stop = %q, want tool", stop) + } +} + +// chatToAnthropic runs Chat Completions stream chunks through the decoder +// and the Anthropic encoder, giving back what an Anthropic client — Claude +// Code among them — is answered for a relay that only speaks Chat. +func chatToAnthropic(t *testing.T, chunks ...string) []map[string]any { + t.Helper() + rec := httptest.NewRecorder() + enc := &anthropicEncoder{w: newSSEWriter(rec), model: "m"} + var d chatDecoder + for _, c := range chunks { + if err := d.decode(c, enc.event); err != nil { + t.Fatal(err) + } + } + enc.finish() + var out []map[string]any + sc := bufio.NewScanner(strings.NewReader(rec.Body.String())) + for sc.Scan() { + if data, ok := strings.CutPrefix(sc.Text(), "data: "); ok { + var m map[string]any + if json.Unmarshal([]byte(data), &m) == nil { + out = append(out, m) + } + } + } + return out +} + +// The same repeated id, seen by an Anthropic client: one content block +// holding the whole call, not one block per fragment with the arguments +// split between them, none of them valid JSON. +func TestRepeatedToolCallIDOpensOneBlock(t *testing.T) { + out := chatToAnthropic(t, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","type":"function","function":{"name":"get_weather","arguments":""}}]}}]}`, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"arguments":"{\"city\":"}}]}}]}`, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"arguments":"\"SF\"}"}}]}}]}`, + `{"id":"x","choices":[{"delta":{},"finish_reason":"tool_calls"}]}`, + ) + var blocks []map[string]any + var input string + for _, ev := range out { + switch ev["type"] { + case "content_block_start": + if b, ok := ev["content_block"].(map[string]any); ok && b["type"] == "tool_use" { + blocks = append(blocks, b) + } + case "content_block_delta": + if d, ok := ev["delta"].(map[string]any); ok && d["type"] == "input_json_delta" { + input += d["partial_json"].(string) + } + } + } + if len(blocks) != 1 { + t.Fatalf("%d tool_use blocks, want 1: %+v", len(blocks), blocks) + } + if blocks[0]["id"] != "call_1" || blocks[0]["name"] != "get_weather" { + t.Fatalf("block = %v", blocks[0]) + } + var args map[string]any + if json.Unmarshal([]byte(input), &args) != nil { + t.Fatalf("arguments %q are not one whole JSON object", input) + } + if args["city"] != "SF" { + t.Fatalf("arguments = %v", args) + } +} + +// Two calls in one reply are still two, whatever a relay repeats: the +// second index opens the second call, and its own fragments continue it. +func TestRepeatedToolCallIDKeepsParallelCalls(t *testing.T) { + out := chatToResponses(t, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_a","type":"function","function":{"name":"one","arguments":""}}]}}]}`, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"call_a","function":{"arguments":"{\"n\":1}"}}]}}]}`, + `{"id":"x","choices":[{"index":0,"delta":{"tool_calls":[{"index":1,"id":"call_b","type":"function","function":{"name":"two","arguments":"{\"n\":2}"}}]}}]}`, + `{"id":"x","choices":[{"delta":{},"finish_reason":"tool_calls"}]}`, + ) + var calls []string + for _, ev := range out { + if ev["type"] != "response.output_item.done" { + continue + } + item, _ := ev["item"].(map[string]any) + if item["type"] != "function_call" { + continue + } + calls = append(calls, item["call_id"].(string)+"="+item["arguments"].(string)) + } + if strings.Join(calls, " ") != `call_a={"n":1} call_b={"n":2}` { + t.Fatalf("calls = %v", calls) + } +}