From 6736995289e2dd180c9e55fea1081123291f2d92 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 10:31:38 +0000 Subject: [PATCH] Port AGUI parallel tool call rendering fixes from .NET #6009 Fix two bugs that prevent parallel tool calls from rendering correctly in AG-UI protocol clients: Bug #2: Make ToolCallResultEvent.MessageId deterministically unique using result-{callId} format. Multiple tool results in the same update batch previously shared a single generated message ID, collapsing them in FE reconciliation. Bug #3: Coalesce consecutive assistant-with-tool-calls messages in toAgentMessages. The AG-UI client creates one assistant message per tool call when parentMessageId is absent. Sending them as separate messages to the LLM triggers HTTP 400 because OpenAI requires tool results to immediately follow every tool_call_id in an assistant message. Also removes the now-dead hasFunctionResultContent helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- agent/hosting/aguihosting/agui_test.go | 120 ++++++++++++++++++++++++- agent/hosting/aguihosting/convert.go | 35 +++++++- agent/hosting/aguihosting/events.go | 31 ++----- 3 files changed, 156 insertions(+), 30 deletions(-) diff --git a/agent/hosting/aguihosting/agui_test.go b/agent/hosting/aguihosting/agui_test.go index 0406224a..ff1c707f 100644 --- a/agent/hosting/aguihosting/agui_test.go +++ b/agent/hosting/aguihosting/agui_test.go @@ -290,8 +290,8 @@ func TestHandler_UnknownDataContent_UsesCurrentMessageLifecycle(t *testing.T) { } // TestHandler_ToolResult_HasDistinctMessageID verifies that tool result events get a -// distinct message ID from the preceding text/tool-call message to avoid AG-UI -// message ID collisions (mirrors .NET fix in microsoft/agent-framework#5800). +// deterministic message ID in "result-{callID}" format, distinct from the preceding +// text/tool-call message (mirrors .NET fix in microsoft/agent-framework#5800 and #6009). func TestHandler_ToolResult_HasDistinctMessageID(t *testing.T) { a := newTestAgent(func(_ context.Context, _ []*message.Message, _ ...agent.Option) iter.Seq2[*agent.ResponseUpdate, error] { return func(yield func(*agent.ResponseUpdate, error) bool) { @@ -327,7 +327,6 @@ func TestHandler_ToolResult_HasDistinctMessageID(t *testing.T) { } // Extract the tool result messageId and the text message start messageId. - // They must differ so that the tool result does not collide with the text message. var toolResultMsgID, textStartMsgID string for _, line := range strings.Split(content, "\n") { if !strings.HasPrefix(line, "data:") { @@ -352,7 +351,122 @@ func TestHandler_ToolResult_HasDistinctMessageID(t *testing.T) { if toolResultMsgID == "" { t.Fatal("expected TOOL_CALL_RESULT event") } + // Tool result message ID must be "result-{callID}" — deterministic and distinct. + if toolResultMsgID != "result-call-1" { + t.Fatalf("tool result message ID = %q, want %q", toolResultMsgID, "result-call-1") + } if textStartMsgID == toolResultMsgID { t.Fatalf("tool result message ID %q must differ from text message ID %q", toolResultMsgID, textStartMsgID) } } + +// TestHandler_ParallelToolResults_HaveUniqueMessageIDs verifies that parallel tool +// results each get a unique deterministic message ID (result-{callID}), so the FE +// does not collapse them in React reconciliation (mirrors .NET fix #6009 Bug #2). +func TestHandler_ParallelToolResults_HaveUniqueMessageIDs(t *testing.T) { + a := newTestAgent(func(_ context.Context, _ []*message.Message, _ ...agent.Option) iter.Seq2[*agent.ResponseUpdate, error] { + return func(yield func(*agent.ResponseUpdate, error) bool) { + yield(&agent.ResponseUpdate{ + Role: message.RoleTool, + Contents: message.Contents{ + &message.FunctionResultContent{CallID: "c1", Result: "result1"}, + &message.FunctionResultContent{CallID: "c2", Result: "result2"}, + }, + }, nil) + } + }) + h := aguihosting.NewJSONHTTPHandler(aguihosting.HandlerConfig{Agent: a}) + + body := `{"threadId":"t1","runId":"r1","messages":[]}` + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + content := rr.Body.String() + + var resultMsgIDs []string + for _, line := range strings.Split(content, "\n") { + if !strings.HasPrefix(line, "data:") { + continue + } + data := strings.TrimPrefix(line, "data: ") + var evt map[string]any + if err := json.Unmarshal([]byte(data), &evt); err != nil { + continue + } + if evt["type"] == "TOOL_CALL_RESULT" { + id, _ := evt["messageId"].(string) + resultMsgIDs = append(resultMsgIDs, id) + } + } + + if len(resultMsgIDs) != 2 { + t.Fatalf("expected 2 TOOL_CALL_RESULT events, got %d", len(resultMsgIDs)) + } + if resultMsgIDs[0] != "result-c1" { + t.Errorf("first result messageId = %q, want %q", resultMsgIDs[0], "result-c1") + } + if resultMsgIDs[1] != "result-c2" { + t.Errorf("second result messageId = %q, want %q", resultMsgIDs[1], "result-c2") + } + if resultMsgIDs[0] == resultMsgIDs[1] { + t.Errorf("parallel tool result message IDs must be unique, both = %q", resultMsgIDs[0]) + } +} + +// TestHandler_ConsecutiveAssistantToolCallMessages_Coalesced verifies that consecutive +// assistant messages that each carry tool calls are merged into one message before being +// forwarded to the agent. The AG-UI client produces one assistant message per tool call +// when parentMessageId is absent; sending them separately would trigger HTTP 400 from +// OpenAI because tool_call_ids must be immediately followed by tool results. +// Mirrors .NET fix in microsoft/agent-framework#6009 Bug #3. +func TestHandler_ConsecutiveAssistantToolCallMessages_Coalesced(t *testing.T) { + var receivedMessages []*message.Message + a := newTestAgent(func(_ context.Context, msgs []*message.Message, _ ...agent.Option) iter.Seq2[*agent.ResponseUpdate, error] { + receivedMessages = msgs + return func(yield func(*agent.ResponseUpdate, error) bool) {} + }) + h := aguihosting.NewJSONHTTPHandler(aguihosting.HandlerConfig{Agent: a}) + + // Two consecutive assistant messages, each with one tool call — simulates what the + // AG-UI client sends after Bug #1 is fixed on the streaming side. + body := `{ + "threadId":"t1","runId":"r1", + "messages":[ + {"id":"u1","role":"user","content":"do both"}, + {"id":"a1","role":"assistant","content":"","toolCalls":[{"id":"c1","type":"function","function":{"name":"tool_a","arguments":"{}"}}]}, + {"id":"a2","role":"assistant","content":"","toolCalls":[{"id":"c2","type":"function","function":{"name":"tool_b","arguments":"{}"}}]}, + {"id":"r1","role":"tool","content":"res1","toolCallId":"c1"}, + {"id":"r2","role":"tool","content":"res2","toolCallId":"c2"} + ] + }` + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + // Find the coalesced assistant message. + var assistantMsgs []*message.Message + for _, m := range receivedMessages { + if m.Role == message.RoleAssistant { + assistantMsgs = append(assistantMsgs, m) + } + } + + if len(assistantMsgs) != 1 { + t.Fatalf("expected 1 assistant message after coalescing, got %d", len(assistantMsgs)) + } + + // The coalesced message must contain both tool calls. + var callIDs []string + for _, c := range assistantMsgs[0].Contents { + if fcc, ok := c.(*message.FunctionCallContent); ok { + callIDs = append(callIDs, fcc.CallID) + } + } + if len(callIDs) != 2 { + t.Fatalf("expected 2 tool calls in coalesced message, got %d", len(callIDs)) + } + if callIDs[0] != "c1" || callIDs[1] != "c2" { + t.Errorf("coalesced tool call IDs = %v, want [c1 c2]", callIDs) + } +} diff --git a/agent/hosting/aguihosting/convert.go b/agent/hosting/aguihosting/convert.go index d28334de..3162c398 100644 --- a/agent/hosting/aguihosting/convert.go +++ b/agent/hosting/aguihosting/convert.go @@ -14,13 +14,46 @@ import ( func toAgentMessages(agMessages []aguiTypes.Message) ([]*message.Message, error) { result := make([]*message.Message, 0, len(agMessages)) + + // Coalesce consecutive assistant messages that carry tool calls into one. + // The AG-UI client creates a separate assistant message per tool call when + // ToolCallStartEvent.parentMessageId is empty, but OpenAI requires every + // assistant message with tool_calls to be immediately followed by tool + // responses for each of its tool_call_ids. Two consecutive single-tool-call + // assistant messages without intervening tool results trigger HTTP 400. + var pendingMsg *message.Message + + flush := func() { + if pendingMsg != nil { + result = append(result, pendingMsg) + pendingMsg = nil + } + } + for _, m := range agMessages { + isAssistantWithToolCalls := m.Role == aguiTypes.RoleAssistant && len(m.ToolCalls) > 0 + + if !isAssistantWithToolCalls { + flush() + } + converted, err := toAgentMessage(m) if err != nil { return nil, err } - result = append(result, converted) + + if isAssistantWithToolCalls { + if pendingMsg == nil { + pendingMsg = converted + } else { + pendingMsg.Contents = append(pendingMsg.Contents, converted.Contents...) + } + } else { + result = append(result, converted) + } } + flush() + return result, nil } diff --git a/agent/hosting/aguihosting/events.go b/agent/hosting/aguihosting/events.go index 18f25f75..2ecfc866 100644 --- a/agent/hosting/aguihosting/events.go +++ b/agent/hosting/aguihosting/events.go @@ -103,13 +103,6 @@ func updatesToAGUIEvents( msgID = aguiEvents.GenerateMessageID() } - // Tool result events must not share the same message ID as the preceding - // text/tool-call message to avoid AG-UI message ID collisions (#5800). - toolResultMsgID := msgID - if hasFunctionResultContent(update.Contents) { - toolResultMsgID = aguiEvents.GenerateMessageID() - } - if currentReasoningMsgID != "" && currentReasoningMsgID != msgID { if !yield(aguiEvents.NewReasoningMessageEndEvent(currentReasoningMsgID), nil) { return @@ -170,11 +163,7 @@ func updatesToAGUIEvents( } continue } - contentMsgID := msgID - if _, ok := c.(*message.FunctionResultContent); ok { - contentMsgID = toolResultMsgID - } - events, convErr := contentToEvents(c, contentMsgID) + events, convErr := contentToEvents(c, msgID) if convErr != nil { if !yield(aguiEvents.NewRunErrorEvent(convErr.Error(), aguiEvents.WithRunID(runID)), convErr) { return @@ -203,15 +192,6 @@ func updatesToAGUIEvents( } } -func hasFunctionResultContent(contents message.Contents) bool { - for _, c := range contents { - if _, ok := c.(*message.FunctionResultContent); ok { - return true - } - } - return false -} - func hasTextLikeContent(contents message.Contents) bool { for _, c := range contents { text, ok := c.(*message.TextContent) @@ -266,11 +246,10 @@ func contentToEvents(content message.Content, messageID string) ([]aguiEvents.Ev if err != nil { return nil, err } - resultMessageID := messageID - if resultMessageID == "" { - resultMessageID = callID - } - return []aguiEvents.Event{aguiEvents.NewToolCallResultEvent(resultMessageID, callID, contentStr)}, nil + // Use result-{callID} so each tool result has a deterministically unique + // message ID. MEAI batches all results under a shared MessageId, which + // collapses them in FE reconciliation when the same id is reused. + return []aguiEvents.Event{aguiEvents.NewToolCallResultEvent("result-"+callID, callID, contentStr)}, nil case *message.DataContent: return dataContentToEvents(c, messageID) default: