fix: stop silently clobbering tool execution decisions on name collisions

haystack/human_in_the_loop/strategies.py

@@ -544,8 +544,18 @@ def _apply_tool_execution_decisions(
     decision_by_name = {d.tool_name: d for d in tool_execution_decisions if d.tool_name}
 
     # Known limitation: If tool calls are missing IDs, we rely on tool names to match decisions to tool calls.
-    # This can lead to incorrect matches if there are multiple tool calls in the provided messages with duplicate names.
-    if not decision_by_id and len(decision_by_name) < len(tool_execution_decisions):
+    # This can lead to incorrect matches if there are multiple tool calls in the provided messages with duplicate
+    # names. `decision_by_name` is built from *all* decisions, so a name collision corrupts the name-based lookup
+    # for an id-less decision even if the colliding decision has an id (it's still the id-less one that depends on
+    # the now-ambiguous `decision_by_name` entry). So we check name duplicates across the whole batch, but only
+    # raise if at least one decision sharing that name is missing a tool_call_id.
+    name_occurrences: dict[str, int] = {}
+    for decision in tool_execution_decisions:
+        name_occurrences[decision.tool_name] = name_occurrences.get(decision.tool_name, 0) + 1
+
+    if any(
+        name_occurrences[decision.tool_name] > 1 for decision in tool_execution_decisions if not decision.tool_call_id
+    ):
         raise ValueError(
             "ToolExecutionDecisions are missing tool_call_id fields and there are multiple tool calls with the same "
             "name. When multiple tool calls with the same name are present, tool_call_id is required to correctly "

releasenotes/notes/fix-hitl-duplicate-name-decision-clobbering-ee2b956add710394.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixed ``human_in_the_loop``'s ``_apply_tool_execution_decisions`` silently applying the wrong
+    ``ToolExecutionDecision`` to a tool call when multiple tool calls shared the same tool name but lacked a
+    ``tool_call_id``. Previously, the ambiguity check was skipped if *any* unrelated tool call in the same batch
+    happened to carry an id, allowing the wrong decision (and therefore the wrong arguments) to be silently
+    applied. The check now correctly raises a ``ValueError`` whenever a tool name collision affects an id-less
+    decision, regardless of other unrelated tool calls in the batch.

test/human_in_the_loop/test_strategies.py

@@ -357,6 +357,77 @@ def test_two_teds_same_name_no_ids(self):
                 ],
             )
 
+    def test_two_teds_same_name_no_ids_plus_unrelated_call_with_id(self):
+        # Same setup as test_two_teds_same_name_no_ids, but with an extra, unrelated tool call that *does* have an
+        # id. The presence of that unrelated id must not bypass the ambiguity check for the id-less duplicates.
+        message_with_tool_calls = ChatMessage.from_assistant(
+            tool_calls=[
+                ToolCall(tool_name="add_database_tool", arguments={"name": "Malte"}),
+                ToolCall(tool_name="add_database_tool", arguments={"name": "Milos"}),
+                ToolCall(tool_name="other_tool", arguments={}, id="xyz"),
+            ]
+        )
+        with pytest.raises(
+            ValueError,
+            match="ToolExecutionDecisions are missing tool_call_id fields and there are multiple tool calls with the "
+            "same name",
+        ):
+            _apply_tool_execution_decisions(
+                tool_call_messages=[message_with_tool_calls],
+                tool_execution_decisions=[
+                    ToolExecutionDecision(
+                        tool_name="add_database_tool", execute=True, final_tool_params={"name": "Malte"}
+                    ),
+                    ToolExecutionDecision(
+                        tool_name="add_database_tool", execute=True, final_tool_params={"name": "Milos"}
+                    ),
+                    ToolExecutionDecision(
+                        tool_name="other_tool", execute=True, tool_call_id="xyz", final_tool_params={}
+                    ),
+                ],
+            )
+
+    def test_two_teds_same_name_one_with_id_one_without(self):
+        # One of the two same-named tool calls has an id, the other doesn't. The id-less one still falls back to
+        # name-based matching against `decision_by_name`, which is corrupted by the name collision, so this must
+        # also be treated as ambiguous even though only one of the two decisions lacks an id.
+        message_with_tool_calls = ChatMessage.from_assistant(
+            tool_calls=[
+                ToolCall(tool_name="search", arguments={"q": "a"}),
+                ToolCall(tool_name="search", arguments={"q": "b"}, id="1"),
+            ]
+        )
+        with pytest.raises(
+            ValueError,
+            match="ToolExecutionDecisions are missing tool_call_id fields and there are multiple tool calls with the "
+            "same name",
+        ):
+            _apply_tool_execution_decisions(
+                tool_call_messages=[message_with_tool_calls],
+                tool_execution_decisions=[
+                    ToolExecutionDecision(tool_name="search", execute=True, final_tool_params={"q": "a"}),
+                    ToolExecutionDecision(
+                        tool_name="search", execute=True, tool_call_id="1", final_tool_params={"q": "b"}
+                    ),
+                ],
+            )
+
+    def test_ted_without_id_unique_name_applies_normally(self):
+        # An id-less decision whose tool name is unique is not ambiguous, so the guard must not raise: name-based
+        # matching is unambiguous here. The decision should be applied normally.
+        message_with_tool_calls = ChatMessage.from_assistant(
+            tool_calls=[ToolCall(tool_name="search", arguments={"q": "a"})]
+        )
+        rejection_messages, new_tool_call_messages = _apply_tool_execution_decisions(
+            tool_call_messages=[message_with_tool_calls],
+            tool_execution_decisions=[
+                ToolExecutionDecision(tool_name="search", execute=True, final_tool_params={"q": "a"})
+            ],
+        )
+        assert rejection_messages == []
+        assert len(new_tool_call_messages) == 1
+        assert new_tool_call_messages[0].tool_calls == [ToolCall(tool_name="search", arguments={"q": "a"})]
+
 
 class TestUpdateChatHistory:
     @pytest.fixture