diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index e8034d32652b..eceee9e0a7da 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -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 diff --git a/tests/agent/test_empty_tool_name_loop_dampening.py b/tests/agent/test_empty_tool_name_loop_dampening.py index ba827e215d79..52222fa2de7b 100644 --- a/tests/agent/test_empty_tool_name_loop_dampening.py +++ b/tests/agent/test_empty_tool_name_loop_dampening.py @@ -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 diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 0dc7a12045fa..f15c611a5880 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -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