human_in_the_loop: duplicate tool-call-name ambiguity guard silently skipped, causing wrong decisions to be applied

[immuhammadfurqan]

Describe the bug

_apply_tool_execution_decisions (in haystack/human_in_the_loop/strategies.py) is supposed to refuse to proceed when it can't unambiguously match a ToolExecutionDecision to a tool call — specifically, when multiple tool calls share the same tool name but have no tool_call_id. There's already a test for this (test_two_teds_same_name_no_ids), and it works only when every tool call/decision in the batch lacks an id.

The guard is:

decision_by_id = {d.tool_call_id: d for d in tool_execution_decisions if d.tool_call_id}
decision_by_name = {d.tool_name: d for d in tool_execution_decisions if d.tool_name}

if not decision_by_id and len(decision_by_name) < len(tool_execution_decisions):
    raise ValueError(...)

The problem: not decision_by_id is checked across the entire batch. If any unrelated tool call elsewhere in the same batch happens to carry an id, decision_by_id becomes non-empty, and the whole guard is skipped — even though the other tool calls (the ones with duplicate names and no ids) are just as ambiguous as before. Once skipped, decision_by_name.get(tc.tool_name) returns whichever decision happens to be last in the dict, and that decision is silently applied to the wrong tool call. This is silent data corruption, not just a missed validation — there's no error, no warning, just a tool getting invoked with someone else's arguments.

Error message

No error is raised — that's the bug. Expected: ValueError: ToolExecutionDecisions are missing tool_call_id fields and there are multiple tool calls with the same name.... Actual: silent success with corrupted arguments.

Expected behavior

The ambiguity check should fire whenever there's a duplicate tool name among the calls/decisions that lack an id, regardless of whether other, unrelated tool calls in the same batch happen to have ids.

To Reproduce

from haystack.dataclasses import ChatMessage, ToolCall
from haystack.human_in_the_loop import ToolExecutionDecision
from haystack.human_in_the_loop.strategies import _apply_tool_execution_decisions

message_with_tool_calls = ChatMessage.from_assistant(
    tool_calls=[
        ToolCall(tool_name="add_database_tool", arguments={"name": "Malte"}),  # no id
        ToolCall(tool_name="add_database_tool", arguments={"name": "Milos"}),  # no id, duplicate name
        ToolCall(tool_name="other_tool", arguments={}, id="xyz"),  # has id, unrelated
    ]
)

rejection_messages, new_tool_call_messages = _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={}),
    ],
)

for m in new_tool_call_messages:
    print(m.tool_calls)
# Both `add_database_tool` calls end up with arguments {'name': 'Milos'}.
# No ValueError is raised. The "Malte" call is silently lost/overwritten.

Additional context

This is the same validation already covered by test_two_teds_same_name_no_ids in test/human_in_the_loop/test_strategies.py, but that test only has tool calls without ids at all. The bug only shows up with a mix of id-bearing and id-less calls in the same batch, which isn't currently tested.

I'd like to fix the guard to check ambiguity only among the id-less subset, and add a regression test for the mixed case, then submit a PR.

System:

• OS: Windows
• Haystack version: main

[sjrl]

Hey @immuhammadfurqan just for more context is this an issue you ran into yourself?

Would be nice to understand when it's possible for tool calls in one batch to have a mixture of IDs and no IDs.

[immuhammadfurqan]

Hey @sjrl — honest answer: no, I didn't hit this in a running pipeline. I found it while auditing _apply_tool_execution_decisions (the same read-through that turned up #11755), so it's a latent/defensive issue rather than something a user reported.

On when a batch can mix ids and no-ids: a single generator won't produce that — OpenAI, HF-API and the Responses generator always set ToolCall.id, and HuggingFaceLocalChatGenerator never does, so its output is uniformly id-less (and the existing test_two_teds_same_name_no_ids already covers that uniform case). A genuine mix only shows up when the tool-call messages are constructed manually or assembled from more than one source before being handed to the HITL strategy.

So I don't think this is hot-path today. The reason I still flagged it: the guard exists precisely to refuse ambiguous name-only matching, but because not decision_by_id is evaluated over the whole batch, one unrelated id-bearing call disables it for the id-less duplicates — and the fallback then silently applies one decision's args to the wrong call with no error or warning. Silent wrong-arguments felt worse than either erroring or not having the guard at all.

Totally fine if you'd rather fold this into the v3 Agent changes — if you think the guard's worth keeping, the fix is small (scope the ambiguity check to just the id-less subset + a regression test for the mixed case), and I'm happy to push that to #11758. Your call on whether it's worth it now vs. v3.

[zeweihan]

Great find — and thank you for framing it as "silent data corruption, not a missed validation." That framing is exactly right and I want to underline why this class of bug is so dangerous, because the signature generalizes beyond this one guard.

The corruption output is plausible, not broken. After the guard is skipped, decision_by_name.get(tc.tool_name) returns a valid ToolExecutionDecision — real tool name, real final_tool_params, structurally correct. The downstream tool executes successfully and returns a result. There is no exception, no empty output, no None. The only thing that's wrong is which arguments got bound to which call. In the repro both add_database_tool calls silently end up {'name': 'Milos'} — the "Malte" write didn't error, it was silently overwritten and then executed as if it were always "Milos". This is the "plausible-but-wrong" failure class: the output is indistinguishable from a correct run unless you independently know what the inputs should have been.

Why the batch-level check is the root cause. The guard if not decision_by_id and ... treats the entire batch as the unit of ambiguity. But ambiguity is a property of the id-less subset, not the batch. A single unrelated id-bearing call anywhere in the batch flips decision_by_id non-empty and disarms the guard for all id-less calls — even the ones that are genuinely ambiguous. The fix the reporter proposes (check ambiguity only among the calls/decisions that lack an id) is correct precisely because it narrows the invariant to the subset where it actually holds. The general principle: a uniqueness/ambiguity invariant must be evaluated per-partition (here: partition by "has id" vs "doesn't"), not collapsed into a single batch-wide boolean. A batch-wide boolean is the cheapest possible check and also the easiest to silently disarm.

The human-in-the-loop angle makes this worse, not safer. HITL is sold as the last line of defense for sensitive workflows — the place where a human is supposed to catch mistakes before they're committed. But this bug means the human approved a decision (the ToolExecutionDecision) that was then silently applied to the wrong call. The human reviewed "execute add_database_tool with name=Milos" and that is what ran — but so did "name=Malte", wearing Milos's approved decision like a borrowed credential. The human's approval no longer attaches to the action it was given for. For any domain where HITL exists specifically because the action is hard to reverse (writes, deletions, external sends, filings), a silently-misapplied decision is worse than no HITL at all, because it manufactures a false sense of having been reviewed.

Minimal companion to the fix (provenance stamping). Beyond narrowing the guard to the id-less subset, it would help to record which decision was applied to which call at apply-time — e.g. stamp the resolved (tool_call, decision) binding (by index or by a generated synthetic id for id-less calls) so that a post-hoc audit can detect that two calls received the same decision. Right now, once the silent overwrite happens, the original "Malte" binding leaves no trace — there's nothing to diff against. Even a debug-level log of applied decision D to call C (matched by: id|name|fallback) would turn this from "silent success with corrupted arguments" into "success with a recoverable mismatch signal."

On the repro/test side: the mixed-batch case (some calls with ids, some without) is the one worth a regression fixture, as the reporter notes — test_two_teds_same_name_no_ids only covers the all-id-less path and is exactly the test that would have passed while this bug shipped.

[sjrl]

@immuhammadfurqan thanks for the explanation. I would prefer to not add this to Haystack v2 and look into adding it potentially in the v3 Agent changes.

Given that it's not a hot path and that manually constructing tool calls is not a common scenario this is not a high priority fix for us. Lets go ahead and close your open PR and we can leave this issue open to tackle later in Haystack v3.