fix: stop silently clobbering tool execution decisions on name collisions

The ambiguity guard in _apply_tool_execution_decisions only checked
whether *every* decision in the batch lacked a tool_call_id. If any
unrelated tool call in the same batch had an id, the guard was
skipped entirely, even though id-less duplicate-named calls were
still present. The matching fallback (decision_by_name) would then
silently resolve to the wrong decision, applying the wrong arguments
to the wrong tool call with no error.

The guard now checks whether any id-less decision's tool name collides
with another decision anywhere in the batch, regardless of whether
the colliding decision has an id, and raises ValueError in that case.

Fixes #11756

Author: immuhammadfurqan

haystack/human_in_the_loop/strategies.py

@@ -514,8 +514,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

@@ -335,6 +335,61 @@ 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"}
+                    ),
+                ],
+            )
+
 
 class TestUpdateChatHistory:
     @pytest.fixture