gateway: a repeated tool call id continues the call it names, not a new call — some relays repeat a tool call's id on every fragment of a streamed reply, where the OpenAI spec sends it on the first alone. The decoder read any non-empty id as "a new call starts", so one tool call came out of translation as one call per fragment: chatEncoder numbered each of them a call of its own, and anthropicEncoder and responsesEncoder opened a content block and an output item for each, every one holding a slice of the arguments and none of them valid JSON. An agent asking for a tool through such a relay was answered several broken calls instead of the one it made — Claude Code, which asks on the Anthropic API, among them. A fragment now continues the call already open when its index matches and it either names no id or names the id already open; only a new id or a new index starts the next one. (#395)
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.
This commit is contained in:
1 parent
618e51a447
commit
3c67b5ca5e
2 files changed
+144
-5
No files matched your search
@@ -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 <thought>…</thought>: 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})
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user