mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(agent): close tool-result tails on invalid-tool and truncated-tool early returns
Invalid-tool exhaustion and truncated-tool early returns skipped finalize_turn, leaving role=tool transcripts that become tool→user on the next turn for strict providers. Call close_interrupted_tool_sequence before persist on those paths (same as interrupt aborts).
This commit is contained in:
parent
8058e01834
commit
9220c0c0bb
3 changed files with 114 additions and 4 deletions
|
|
@ -2343,13 +2343,17 @@ def run_conversation(
|
|||
force=True,
|
||||
)
|
||||
agent._cleanup_task_resources(effective_task_id)
|
||||
agent._persist_session(messages, conversation_history)
|
||||
_final_response = (
|
||||
"Stream repeatedly dropped mid tool-call (network); "
|
||||
"the tool was not executed"
|
||||
if _is_stub_stall
|
||||
else "Response truncated due to output length limit"
|
||||
)
|
||||
# Prior successful tool batches (or injected tool
|
||||
# errors) can leave a tool-result tail; this path
|
||||
# never reaches finalize_turn (#48879 class).
|
||||
close_interrupted_tool_sequence(messages, _final_response)
|
||||
agent._persist_session(messages, conversation_history)
|
||||
return {
|
||||
"final_response": _final_response,
|
||||
"messages": messages,
|
||||
|
|
@ -5022,8 +5026,13 @@ def run_conversation(
|
|||
agent._flush_status_buffer()
|
||||
agent._vprint(f"{agent.log_prefix}❌ Max retries (3) for invalid tool calls exceeded. Stopping as partial.", force=True)
|
||||
agent._invalid_tool_retries = 0
|
||||
agent._persist_session(messages, conversation_history)
|
||||
_final_response = f"Model generated invalid tool call: {invalid_preview}"
|
||||
# Prior <3 retries (or an earlier successful tool batch)
|
||||
# leave a tool-result tail. Closing it here matches
|
||||
# interrupt aborts (#48879 / #52592) so the next user
|
||||
# turn is not tool→user for strict providers.
|
||||
close_interrupted_tool_sequence(messages, _final_response)
|
||||
agent._persist_session(messages, conversation_history)
|
||||
return {
|
||||
"final_response": _final_response,
|
||||
"messages": messages,
|
||||
|
|
@ -5103,14 +5112,18 @@ def run_conversation(
|
|||
)
|
||||
agent._invalid_json_retries = 0
|
||||
agent._cleanup_task_resources(effective_task_id)
|
||||
_final_response = "Response truncated due to output length limit"
|
||||
# Same tool-tail close as interrupt / invalid-tool
|
||||
# exhaustion — this path never reaches finalize_turn.
|
||||
close_interrupted_tool_sequence(messages, _final_response)
|
||||
agent._persist_session(messages, conversation_history)
|
||||
return {
|
||||
"final_response": "Response truncated due to output length limit",
|
||||
"final_response": _final_response,
|
||||
"messages": messages,
|
||||
"api_calls": api_call_count,
|
||||
"completed": False,
|
||||
"partial": True,
|
||||
"error": "Response truncated due to output length limit",
|
||||
"error": _final_response,
|
||||
}
|
||||
|
||||
# Track retries for invalid JSON arguments
|
||||
|
|
|
|||
|
|
@ -277,6 +277,27 @@ def test_all_invalid_batch_still_strikes_out(agent_env):
|
|||
assert "invalid tool call" in (result.get("error") or "")
|
||||
|
||||
|
||||
def test_invalid_tool_exhaustion_closes_tool_tail(agent_env):
|
||||
"""Invalid-tool 3-strike partial must not leave a durable tool→user tail (#48879 class).
|
||||
|
||||
Retries <3 append assistant+error tool rows, so the transcript already ends
|
||||
on ``tool`` before the exhaustion early-return. That return must close the
|
||||
sequence (same contract as interrupt aborts) so the next user turn is not
|
||||
``tool → user`` for strict providers.
|
||||
"""
|
||||
agent, handler = agent_env
|
||||
for _ in range(3):
|
||||
handler.response_queue.append(_tc_resp("frobnicate_xyz", "{}"))
|
||||
|
||||
result = agent.run_conversation("degenerate", conversation_history=[], task_id="t")
|
||||
|
||||
assert result.get("partial", False)
|
||||
msgs = result.get("messages") or []
|
||||
assert msgs, "expected persisted conversation messages"
|
||||
assert msgs[-1].get("role") == "assistant"
|
||||
assert "invalid tool call" in (msgs[-1].get("content") or "").lower()
|
||||
|
||||
|
||||
def test_mixed_batch_invalid_call_with_broken_json_does_not_retry_turn(agent_env):
|
||||
"""Broken args on a never-executing invalid call must not trigger the JSON retry loop."""
|
||||
agent, handler = agent_env
|
||||
|
|
|
|||
|
|
@ -5688,6 +5688,82 @@ class TestRunConversation:
|
|||
assert "truncated due to output length limit" in result["error"]
|
||||
mock_handle_function_call.assert_not_called()
|
||||
|
||||
def test_truncated_tool_json_after_tool_batch_closes_tool_tail(self, agent):
|
||||
"""finish_reason=tool_calls + truncated args after a real tool must close tool→user."""
|
||||
self._setup_agent(agent)
|
||||
agent.valid_tool_names.add("write_file")
|
||||
good_tc = _mock_tool_call(
|
||||
name="write_file",
|
||||
arguments='{"path":"ok.md","content":"x"}',
|
||||
call_id="c_ok",
|
||||
)
|
||||
good_resp = _mock_response(
|
||||
content="", finish_reason="tool_calls", tool_calls=[good_tc],
|
||||
)
|
||||
bad_tc = _mock_tool_call(
|
||||
name="write_file",
|
||||
arguments='{"path":"report.md","content":"partial',
|
||||
call_id="c_bad",
|
||||
)
|
||||
bad_resp = _mock_response(
|
||||
content="", finish_reason="tool_calls", tool_calls=[bad_tc],
|
||||
)
|
||||
agent.client.chat.completions.create.side_effect = [good_resp, bad_resp]
|
||||
|
||||
with (
|
||||
patch("run_agent.handle_function_call", return_value='{"success":true}'),
|
||||
patch.object(agent, "_persist_session"),
|
||||
patch.object(agent, "_save_trajectory"),
|
||||
patch.object(agent, "_cleanup_task_resources"),
|
||||
):
|
||||
result = agent.run_conversation("write then truncate")
|
||||
|
||||
assert result.get("partial") is True
|
||||
msgs = result.get("messages") or []
|
||||
assert msgs[-1].get("role") == "assistant"
|
||||
assert "truncated" in (msgs[-1].get("content") or "").lower()
|
||||
assert any(isinstance(m, dict) and m.get("role") == "tool" for m in msgs)
|
||||
|
||||
def test_length_truncated_tool_exhaustion_after_tool_batch_closes_tool_tail(self, agent):
|
||||
"""Length-handler truncated-tool exhaustion after a tool batch must close tool→user."""
|
||||
self._setup_agent(agent)
|
||||
agent.valid_tool_names.add("write_file")
|
||||
good_tc = _mock_tool_call(
|
||||
name="write_file",
|
||||
arguments='{"path":"ok.md","content":"x"}',
|
||||
call_id="c_ok",
|
||||
)
|
||||
good_resp = _mock_response(
|
||||
content="", finish_reason="tool_calls", tool_calls=[good_tc],
|
||||
)
|
||||
bad_tc = _mock_tool_call(
|
||||
name="write_file",
|
||||
arguments='{"path":"report.md","content":"partial',
|
||||
call_id="c_bad",
|
||||
)
|
||||
bad_resp = _mock_response(
|
||||
content="", finish_reason="length", tool_calls=[bad_tc],
|
||||
)
|
||||
# One successful tool turn, then 4 truncated retries + 5th exhaustion.
|
||||
agent.client.chat.completions.create.side_effect = [
|
||||
good_resp,
|
||||
bad_resp, bad_resp, bad_resp, bad_resp, bad_resp,
|
||||
]
|
||||
|
||||
with (
|
||||
patch("run_agent.handle_function_call", return_value='{"success":true}'),
|
||||
patch.object(agent, "_persist_session"),
|
||||
patch.object(agent, "_save_trajectory"),
|
||||
patch.object(agent, "_cleanup_task_resources"),
|
||||
):
|
||||
result = agent.run_conversation("write then hit length truncate")
|
||||
|
||||
assert result.get("partial") is True
|
||||
assert "truncated due to output length limit" in (result.get("error") or "")
|
||||
msgs = result.get("messages") or []
|
||||
assert msgs[-1].get("role") == "assistant"
|
||||
assert "truncated" in (msgs[-1].get("content") or "").lower()
|
||||
|
||||
def test_kanban_block_called_on_iteration_exhaustion(self, agent, monkeypatch):
|
||||
"""Regression: kanban worker must signal the dispatcher when its
|
||||
iteration budget is exhausted, otherwise the task silently re-runs
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue