Merge pull request #61 from tinyc0der/fix/kiro-tool-results

fix(kiro): Handle tool results correctly in OpenAI format translation

internal/translator/kiro/common/message_merge.go

@@ -10,6 +10,7 @@ import (
 // MergeAdjacentMessages merges adjacent messages with the same role.
 // This reduces API call complexity and improves compatibility.
 // Based on AIClient-2-API implementation.
+// NOTE: Tool messages are NOT merged because each has a unique tool_call_id that must be preserved.
 func MergeAdjacentMessages(messages []gjson.Result) []gjson.Result {
 	if len(messages) <= 1 {
 		return messages
@@ -26,6 +27,12 @@ func MergeAdjacentMessages(messages []gjson.Result) []gjson.Result {
 		currentRole := msg.Get("role").String()
 		lastRole := lastMsg.Get("role").String()
 
+		// Don't merge tool messages - each has a unique tool_call_id
+		if currentRole == "tool" || lastRole == "tool" {
+			merged = append(merged, msg)
+			continue
+		}
+
 		if currentRole == lastRole {
 			// Merge content from current message into last message
 			mergedContent := mergeMessageContent(lastMsg, msg)

internal/translator/kiro/openai/kiro_openai_request.go

@@ -450,24 +450,10 @@ func processOpenAIMessages(messages gjson.Result, modelID, origin string) ([]Kir
 	// Merge adjacent messages with the same role
 	messagesArray := kirocommon.MergeAdjacentMessages(messages.Array())
 
-	// Build tool_call_id to name mapping from assistant messages
-	toolCallIDToName := make(map[string]string)
-	for _, msg := range messagesArray {
-		if msg.Get("role").String() == "assistant" {
-			toolCalls := msg.Get("tool_calls")
-			if toolCalls.IsArray() {
-				for _, tc := range toolCalls.Array() {
-					if tc.Get("type").String() == "function" {
-						id := tc.Get("id").String()
-						name := tc.Get("function.name").String()
-						if id != "" && name != "" {
-							toolCallIDToName[id] = name
-						}
-					}
-				}
-			}
-		}
-	}
+	// Track pending tool results that should be attached to the next user message
+	// This is critical for LiteLLM-translated requests where tool results appear
+	// as separate "tool" role messages between assistant and user messages
+	var pendingToolResults []KiroToolResult
 
 	for i, msg := range messagesArray {
 		role := msg.Get("role").String()
@@ -480,6 +466,10 @@ func processOpenAIMessages(messages gjson.Result, modelID, origin string) ([]Kir
 
 		case "user":
 			userMsg, toolResults := buildUserMessageFromOpenAI(msg, modelID, origin)
+			// Merge any pending tool results from preceding "tool" role messages
+			toolResults = append(pendingToolResults, toolResults...)
+			pendingToolResults = nil // Reset pending tool results
+
 			if isLastMessage {
 				currentUserMsg = &userMsg
 				currentToolResults = toolResults
@@ -505,6 +495,24 @@ func processOpenAIMessages(messages gjson.Result, modelID, origin string) ([]Kir
 
 		case "assistant":
 			assistantMsg := buildAssistantMessageFromOpenAI(msg)
+
+			// If there are pending tool results, we need to insert a synthetic user message
+			// before this assistant message to maintain proper conversation structure
+			if len(pendingToolResults) > 0 {
+				syntheticUserMsg := KiroUserInputMessage{
+					Content: "Tool results provided.",
+					ModelID: modelID,
+					Origin:  origin,
+					UserInputMessageContext: &KiroUserInputMessageContext{
+						ToolResults: pendingToolResults,
+					},
+				}
+				history = append(history, KiroHistoryMessage{
+					UserInputMessage: &syntheticUserMsg,
+				})
+				pendingToolResults = nil
+			}
+
 			if isLastMessage {
 				history = append(history, KiroHistoryMessage{
 					AssistantResponseMessage: &assistantMsg,
@@ -524,7 +532,7 @@ func processOpenAIMessages(messages gjson.Result, modelID, origin string) ([]Kir
 		case "tool":
 			// Tool messages in OpenAI format provide results for tool_calls
 			// These are typically followed by user or assistant messages
-			// Process them and merge into the next user message's tool results
+			// Collect them as pending and attach to the next user message
 			toolCallID := msg.Get("tool_call_id").String()
 			content := msg.Get("content").String()
 
@@ -534,9 +542,21 @@ func processOpenAIMessages(messages gjson.Result, modelID, origin string) ([]Kir
 					Content:   []KiroTextContent{{Text: content}},
 					Status:    "success",
 				}
-				// Tool results should be included in the next user message
-				// For now, collect them and they'll be handled when we build the current message
-				currentToolResults = append(currentToolResults, toolResult)
+				// Collect pending tool results to attach to the next user message
+				pendingToolResults = append(pendingToolResults, toolResult)
+			}
+		}
+	}
+
+	// Handle case where tool results are at the end with no following user message
+	if len(pendingToolResults) > 0 {
+		currentToolResults = append(currentToolResults, pendingToolResults...)
+		// If there's no current user message, create a synthetic one for the tool results
+		if currentUserMsg == nil {
+			currentUserMsg = &KiroUserInputMessage{
+				Content: "Tool results provided.",
+				ModelID: modelID,
+				Origin:  origin,
 			}
 		}
 	}
@@ -551,9 +571,6 @@ func buildUserMessageFromOpenAI(msg gjson.Result, modelID, origin string) (KiroU
 	var toolResults []KiroToolResult
 	var images []KiroImage
 
-	// Track seen toolCallIds to deduplicate
-	seenToolCallIDs := make(map[string]bool)
-
 	if content.IsArray() {
 		for _, part := range content.Array() {
 			partType := part.Get("type").String()
@@ -589,9 +606,6 @@ func buildUserMessageFromOpenAI(msg gjson.Result, modelID, origin string) (KiroU
 		contentBuilder.WriteString(content.String())
 	}
 
-	// Check for tool_calls in the message (shouldn't be in user messages, but handle edge cases)
-	_ = seenToolCallIDs // Used for deduplication if needed
-
 	userMsg := KiroUserInputMessage{
 		Content: contentBuilder.String(),
 		ModelID: modelID,

internal/translator/kiro/openai/kiro_openai_request_test.go

@@ -0,0 +1,386 @@
+package openai
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+// TestToolResultsAttachedToCurrentMessage verifies that tool results from "tool" role messages
+// are properly attached to the current user message (the last message in the conversation).
+// This is critical for LiteLLM-translated requests where tool results appear as separate messages.
+func TestToolResultsAttachedToCurrentMessage(t *testing.T) {
+	// OpenAI format request simulating LiteLLM's translation from Anthropic format
+	// Sequence: user -> assistant (with tool_calls) -> tool (result) -> user
+	// The last user message should have the tool results attached
+	input := []byte(`{
+		"model": "kiro-claude-opus-4-5-agentic",
+		"messages": [
+			{"role": "user", "content": "Hello, can you read a file for me?"},
+			{
+				"role": "assistant",
+				"content": "I'll read that file for you.",
+				"tool_calls": [
+					{
+						"id": "call_abc123",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{\"file_path\": \"/tmp/test.txt\"}"
+						}
+					}
+				]
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_abc123",
+				"content": "File contents: Hello World!"
+			},
+			{"role": "user", "content": "What did the file say?"}
+		]
+	}`)
+
+	result, _ := BuildKiroPayloadFromOpenAI(input, "kiro-model", "", "CLI", false, false, nil, nil)
+
+	var payload KiroPayload
+	if err := json.Unmarshal(result, &payload); err != nil {
+		t.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	// The last user message becomes currentMessage
+	// History should have: user (first), assistant (with tool_calls)
+	t.Logf("History count: %d", len(payload.ConversationState.History))
+	if len(payload.ConversationState.History) != 2 {
+		t.Errorf("Expected 2 history entries (user + assistant), got %d", len(payload.ConversationState.History))
+	}
+
+	// Tool results should be attached to currentMessage (the last user message)
+	ctx := payload.ConversationState.CurrentMessage.UserInputMessage.UserInputMessageContext
+	if ctx == nil {
+		t.Fatal("Expected currentMessage to have UserInputMessageContext with tool results")
+	}
+
+	if len(ctx.ToolResults) != 1 {
+		t.Fatalf("Expected 1 tool result in currentMessage, got %d", len(ctx.ToolResults))
+	}
+
+	tr := ctx.ToolResults[0]
+	if tr.ToolUseID != "call_abc123" {
+		t.Errorf("Expected toolUseId 'call_abc123', got '%s'", tr.ToolUseID)
+	}
+	if len(tr.Content) == 0 || tr.Content[0].Text != "File contents: Hello World!" {
+		t.Errorf("Tool result content mismatch, got: %+v", tr.Content)
+	}
+}
+
+// TestToolResultsInHistoryUserMessage verifies that when there are multiple user messages
+// after tool results, the tool results are attached to the correct user message in history.
+func TestToolResultsInHistoryUserMessage(t *testing.T) {
+	// Sequence: user -> assistant (with tool_calls) -> tool (result) -> user -> assistant -> user
+	// The first user after tool should have tool results in history
+	input := []byte(`{
+		"model": "kiro-claude-opus-4-5-agentic",
+		"messages": [
+			{"role": "user", "content": "Hello"},
+			{
+				"role": "assistant",
+				"content": "I'll read the file.",
+				"tool_calls": [
+					{
+						"id": "call_1",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{}"
+						}
+					}
+				]
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_1",
+				"content": "File result"
+			},
+			{"role": "user", "content": "Thanks for the file"},
+			{"role": "assistant", "content": "You're welcome"},
+			{"role": "user", "content": "Bye"}
+		]
+	}`)
+
+	result, _ := BuildKiroPayloadFromOpenAI(input, "kiro-model", "", "CLI", false, false, nil, nil)
+
+	var payload KiroPayload
+	if err := json.Unmarshal(result, &payload); err != nil {
+		t.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	// History should have: user, assistant, user (with tool results), assistant
+	// CurrentMessage should be: last user "Bye"
+	t.Logf("History count: %d", len(payload.ConversationState.History))
+
+	// Find the user message in history with tool results
+	foundToolResults := false
+	for i, h := range payload.ConversationState.History {
+		if h.UserInputMessage != nil {
+			t.Logf("History[%d]: user message content=%q", i, h.UserInputMessage.Content)
+			if h.UserInputMessage.UserInputMessageContext != nil {
+				if len(h.UserInputMessage.UserInputMessageContext.ToolResults) > 0 {
+					foundToolResults = true
+					t.Logf("  Found %d tool results", len(h.UserInputMessage.UserInputMessageContext.ToolResults))
+					tr := h.UserInputMessage.UserInputMessageContext.ToolResults[0]
+					if tr.ToolUseID != "call_1" {
+						t.Errorf("Expected toolUseId 'call_1', got '%s'", tr.ToolUseID)
+					}
+				}
+			}
+		}
+		if h.AssistantResponseMessage != nil {
+			t.Logf("History[%d]: assistant message content=%q", i, h.AssistantResponseMessage.Content)
+		}
+	}
+
+	if !foundToolResults {
+		t.Error("Tool results were not attached to any user message in history")
+	}
+}
+
+// TestToolResultsWithMultipleToolCalls verifies handling of multiple tool calls
+func TestToolResultsWithMultipleToolCalls(t *testing.T) {
+	input := []byte(`{
+		"model": "kiro-claude-opus-4-5-agentic",
+		"messages": [
+			{"role": "user", "content": "Read two files for me"},
+			{
+				"role": "assistant",
+				"content": "I'll read both files.",
+				"tool_calls": [
+					{
+						"id": "call_1",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{\"file_path\": \"/tmp/file1.txt\"}"
+						}
+					},
+					{
+						"id": "call_2",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{\"file_path\": \"/tmp/file2.txt\"}"
+						}
+					}
+				]
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_1",
+				"content": "Content of file 1"
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_2",
+				"content": "Content of file 2"
+			},
+			{"role": "user", "content": "What do they say?"}
+		]
+	}`)
+
+	result, _ := BuildKiroPayloadFromOpenAI(input, "kiro-model", "", "CLI", false, false, nil, nil)
+
+	var payload KiroPayload
+	if err := json.Unmarshal(result, &payload); err != nil {
+		t.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	t.Logf("History count: %d", len(payload.ConversationState.History))
+	t.Logf("CurrentMessage content: %q", payload.ConversationState.CurrentMessage.UserInputMessage.Content)
+
+	// Check if there are any tool results anywhere
+	var totalToolResults int
+	for i, h := range payload.ConversationState.History {
+		if h.UserInputMessage != nil && h.UserInputMessage.UserInputMessageContext != nil {
+			count := len(h.UserInputMessage.UserInputMessageContext.ToolResults)
+			t.Logf("History[%d] user message has %d tool results", i, count)
+			totalToolResults += count
+		}
+	}
+
+	ctx := payload.ConversationState.CurrentMessage.UserInputMessage.UserInputMessageContext
+	if ctx != nil {
+		t.Logf("CurrentMessage has %d tool results", len(ctx.ToolResults))
+		totalToolResults += len(ctx.ToolResults)
+	} else {
+		t.Logf("CurrentMessage has no UserInputMessageContext")
+	}
+
+	if totalToolResults != 2 {
+		t.Errorf("Expected 2 tool results total, got %d", totalToolResults)
+	}
+}
+
+// TestToolResultsAtEndOfConversation verifies tool results are handled when
+// the conversation ends with tool results (no following user message)
+func TestToolResultsAtEndOfConversation(t *testing.T) {
+	input := []byte(`{
+		"model": "kiro-claude-opus-4-5-agentic",
+		"messages": [
+			{"role": "user", "content": "Read a file"},
+			{
+				"role": "assistant",
+				"content": "Reading the file.",
+				"tool_calls": [
+					{
+						"id": "call_end",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{\"file_path\": \"/tmp/test.txt\"}"
+						}
+					}
+				]
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_end",
+				"content": "File contents here"
+			}
+		]
+	}`)
+
+	result, _ := BuildKiroPayloadFromOpenAI(input, "kiro-model", "", "CLI", false, false, nil, nil)
+
+	var payload KiroPayload
+	if err := json.Unmarshal(result, &payload); err != nil {
+		t.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	// When the last message is a tool result, a synthetic user message is created
+	// and tool results should be attached to it
+	ctx := payload.ConversationState.CurrentMessage.UserInputMessage.UserInputMessageContext
+	if ctx == nil || len(ctx.ToolResults) == 0 {
+		t.Error("Expected tool results to be attached to current message when conversation ends with tool result")
+	} else {
+		if ctx.ToolResults[0].ToolUseID != "call_end" {
+			t.Errorf("Expected toolUseId 'call_end', got '%s'", ctx.ToolResults[0].ToolUseID)
+		}
+	}
+}
+
+// TestToolResultsFollowedByAssistant verifies handling when tool results are followed
+// by an assistant message (no intermediate user message).
+// This is the pattern from LiteLLM translation of Anthropic format where:
+// user message has ONLY tool_result blocks -> LiteLLM creates tool messages
+// then the next message is assistant
+func TestToolResultsFollowedByAssistant(t *testing.T) {
+	// Sequence: user -> assistant (with tool_calls) -> tool -> tool -> assistant -> user
+	// This simulates LiteLLM's translation of:
+	//   user: "Read files"
+	//   assistant: [tool_use, tool_use]
+	//   user: [tool_result, tool_result]  <- becomes multiple "tool" role messages
+	//   assistant: "I've read them"
+	//   user: "What did they say?"
+	input := []byte(`{
+		"model": "kiro-claude-opus-4-5-agentic",
+		"messages": [
+			{"role": "user", "content": "Read two files for me"},
+			{
+				"role": "assistant",
+				"content": "I'll read both files.",
+				"tool_calls": [
+					{
+						"id": "call_1",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{\"file_path\": \"/tmp/a.txt\"}"
+						}
+					},
+					{
+						"id": "call_2",
+						"type": "function",
+						"function": {
+							"name": "Read",
+							"arguments": "{\"file_path\": \"/tmp/b.txt\"}"
+						}
+					}
+				]
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_1",
+				"content": "Contents of file A"
+			},
+			{
+				"role": "tool",
+				"tool_call_id": "call_2",
+				"content": "Contents of file B"
+			},
+			{
+				"role": "assistant",
+				"content": "I've read both files."
+			},
+			{"role": "user", "content": "What did they say?"}
+		]
+	}`)
+
+	result, _ := BuildKiroPayloadFromOpenAI(input, "kiro-model", "", "CLI", false, false, nil, nil)
+
+	var payload KiroPayload
+	if err := json.Unmarshal(result, &payload); err != nil {
+		t.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	t.Logf("History count: %d", len(payload.ConversationState.History))
+
+	// Tool results should be attached to a synthetic user message or the history should be valid
+	var totalToolResults int
+	for i, h := range payload.ConversationState.History {
+		if h.UserInputMessage != nil {
+			t.Logf("History[%d]: user message content=%q", i, h.UserInputMessage.Content)
+			if h.UserInputMessage.UserInputMessageContext != nil {
+				count := len(h.UserInputMessage.UserInputMessageContext.ToolResults)
+				t.Logf("  Has %d tool results", count)
+				totalToolResults += count
+			}
+		}
+		if h.AssistantResponseMessage != nil {
+			t.Logf("History[%d]: assistant message content=%q", i, h.AssistantResponseMessage.Content)
+		}
+	}
+
+	ctx := payload.ConversationState.CurrentMessage.UserInputMessage.UserInputMessageContext
+	if ctx != nil {
+		t.Logf("CurrentMessage has %d tool results", len(ctx.ToolResults))
+		totalToolResults += len(ctx.ToolResults)
+	}
+
+	if totalToolResults != 2 {
+		t.Errorf("Expected 2 tool results total, got %d", totalToolResults)
+	}
+}
+
+// TestAssistantEndsConversation verifies handling when assistant is the last message
+func TestAssistantEndsConversation(t *testing.T) {
+	input := []byte(`{
+		"model": "kiro-claude-opus-4-5-agentic",
+		"messages": [
+			{"role": "user", "content": "Hello"},
+			{
+				"role": "assistant",
+				"content": "Hi there!"
+			}
+		]
+	}`)
+
+	result, _ := BuildKiroPayloadFromOpenAI(input, "kiro-model", "", "CLI", false, false, nil, nil)
+
+	var payload KiroPayload
+	if err := json.Unmarshal(result, &payload); err != nil {
+		t.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	// When assistant is last, a "Continue" user message should be created
+	if payload.ConversationState.CurrentMessage.UserInputMessage.Content == "" {
+		t.Error("Expected a 'Continue' message to be created when assistant is last")
+	}
+}