diff --git a/internal/agent/loop_history.go b/internal/agent/loop_history.go index fe8132c7..a1fc9321 100644 --- a/internal/agent/loop_history.go +++ b/internal/agent/loop_history.go @@ -504,7 +504,8 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { return nil, dropped } - // 2. Walk through messages ensuring tool_result follows matching tool_use. + // 2. Walk through messages ensuring tool_result follows matching tool_use + // and that roles alternate correctly (user↔assistant). // Also dedup tool call IDs across the transcript for legacy sessions that // may have persisted duplicates before the live uniquify fix was deployed. var result []providers.Message @@ -524,18 +525,26 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { // results with the same raw ID pair correctly in encounter order. idQueue := make(map[string][]string, len(msg.ToolCalls)) // origID → []newID expectedIDs := make(map[string]bool, len(msg.ToolCalls)) + didDedup := false for j := range msg.ToolCalls { origID := msg.ToolCalls[j].ID newID := origID if globalSeen[origID] { newID = fmt.Sprintf("%s_dedup_%d", origID, j) slog.Debug("sanitizeHistory: dedup tool call ID", "orig", origID, "new", newID) + didDedup = true + dropped++ // count as change so cleaned history is persisted back to DB } msg.ToolCalls[j].ID = newID globalSeen[newID] = true idQueue[origID] = append(idQueue[origID], newID) expectedIDs[newID] = true } + // When dedup rewrites IDs, clear RawAssistantContent so the provider + // uses the corrected ToolCalls instead of raw JSON with stale IDs. + if didDedup { + msg.RawAssistantContent = nil + } result = append(result, msg) @@ -578,6 +587,28 @@ func sanitizeHistory(msgs []providers.Message) ([]providers.Message, int) { } } + // 3. Fix role alternation: LLM APIs require user↔assistant alternation. + // Merge consecutive same-role messages (e.g. two user messages) into one, + // which can happen from bootstrap nudges, inject channel, or session corruption. + if len(result) > 1 { + merged := make([]providers.Message, 0, len(result)) + merged = append(merged, result[0]) + for j := 1; j < len(result); j++ { + prev := &merged[len(merged)-1] + curr := result[j] + // Only merge plain messages (no tool_calls, no tool role) + if curr.Role == prev.Role && curr.Role != "tool" && len(curr.ToolCalls) == 0 && len(prev.ToolCalls) == 0 { + slog.Debug("sanitizeHistory: merging consecutive same-role messages", + "role", curr.Role, "index", j) + prev.Content += "\n\n" + curr.Content + dropped++ + } else { + merged = append(merged, curr) + } + } + result = merged + } + return result, dropped } diff --git a/internal/agent/loop_history_test.go b/internal/agent/loop_history_test.go index 33290ff0..0f36a072 100644 --- a/internal/agent/loop_history_test.go +++ b/internal/agent/loop_history_test.go @@ -257,8 +257,8 @@ func TestSanitizeHistory_DedupsDuplicateIDsWithinTurn(t *testing.T) { {Role: "assistant", Content: "done"}, } got, dropped := sanitizeHistory(msgs) - if dropped != 0 { - t.Errorf("expected 0 dropped, got %d", dropped) + if dropped != 1 { + t.Errorf("expected 1 dropped (dedup counts as change), got %d", dropped) } // Both tool results must be present and paired correctly @@ -326,6 +326,90 @@ func TestSanitizeHistory_NoDedupWhenIDsUnique(t *testing.T) { } } +func TestSanitizeHistory_MergesConsecutiveUserMessages(t *testing.T) { + msgs := []providers.Message{ + {Role: "user", Content: "hello"}, + {Role: "user", Content: "world"}, + {Role: "assistant", Content: "hi there"}, + } + got, dropped := sanitizeHistory(msgs) + if len(got) != 2 { + t.Fatalf("expected 2 messages after merge, got %d", len(got)) + } + if got[0].Content != "hello\n\nworld" { + t.Errorf("expected merged content, got %q", got[0].Content) + } + if dropped != 1 { + t.Errorf("expected 1 dropped, got %d", dropped) + } +} + +func TestSanitizeHistory_MergesConsecutiveAssistantMessages(t *testing.T) { + msgs := []providers.Message{ + {Role: "user", Content: "question"}, + {Role: "assistant", Content: "part 1"}, + {Role: "assistant", Content: "part 2"}, + {Role: "user", Content: "follow-up"}, + } + got, dropped := sanitizeHistory(msgs) + if len(got) != 3 { + t.Fatalf("expected 3 messages after merge, got %d", len(got)) + } + if got[1].Content != "part 1\n\npart 2" { + t.Errorf("expected merged assistant content, got %q", got[1].Content) + } + if dropped != 1 { + t.Errorf("expected 1 dropped, got %d", dropped) + } +} + +func TestSanitizeHistory_DedupClearsRawAssistantContent(t *testing.T) { + // When dedup rewrites tool call IDs, RawAssistantContent (which has the old IDs) + // must be cleared so the provider uses the corrected ToolCalls. + rawContent := []byte(`[{"type":"text","text":"ok"},{"type":"tool_use","id":"tool_1","name":"search","input":{}}]`) + msgs := []providers.Message{ + {Role: "user", Content: "first"}, + {Role: "assistant", ToolCalls: []providers.ToolCall{{ID: "tool_1", Name: "search"}}, RawAssistantContent: rawContent}, + {Role: "tool", Content: "result1", ToolCallID: "tool_1"}, + {Role: "assistant", Content: "ok"}, + {Role: "user", Content: "second"}, + // Second turn has same tool_1 ID — triggers dedup + {Role: "assistant", ToolCalls: []providers.ToolCall{{ID: "tool_1", Name: "search"}}, RawAssistantContent: rawContent}, + {Role: "tool", Content: "result2", ToolCallID: "tool_1"}, + {Role: "assistant", Content: "done"}, + } + got, _ := sanitizeHistory(msgs) + // First assistant should keep RawAssistantContent (no dedup needed) + if got[1].RawAssistantContent == nil { + t.Error("first assistant should keep RawAssistantContent") + } + // Second assistant (index 5) should have RawAssistantContent cleared due to dedup + if got[5].RawAssistantContent != nil { + t.Error("dedup'd assistant should have RawAssistantContent cleared") + } + // Tool result for second turn should have dedup'd ID + if got[6].ToolCallID == "tool_1" { + t.Errorf("expected dedup'd tool_call_id, got %q", got[6].ToolCallID) + } +} + +func TestSanitizeHistory_NoMergeForToolCallMessages(t *testing.T) { + // Assistant messages WITH tool_calls should NOT be merged + msgs := []providers.Message{ + {Role: "user", Content: "do something"}, + {Role: "assistant", Content: "ok", ToolCalls: []providers.ToolCall{{ID: "tc1", Name: "test"}}}, + {Role: "tool", Content: "result", ToolCallID: "tc1"}, + {Role: "assistant", Content: "done"}, + } + got, dropped := sanitizeHistory(msgs) + if len(got) != 4 { + t.Fatalf("expected 4 messages (no merge), got %d", len(got)) + } + if dropped != 0 { + t.Errorf("expected 0 dropped, got %d", dropped) + } +} + func TestUniquifyToolCallIDs(t *testing.T) { runID := "abcdef12-3456-7890-abcd-ef1234567890"