mirror of
https://github.com/agent0ai/agent-zero.git
synced 2026-07-26 01:03:33 +00:00
fix(response): keep completions on the tool path
Route native Responses output text through the normalized response-tool executor instead of ending the monologue directly. Preserve active goal overrides and Responses state cleanup, wait for complete native streams, and reject empty response payloads.
This commit is contained in:
parent
391fab9469
commit
c6136da08b
9 changed files with 240 additions and 77 deletions
|
|
@ -28,6 +28,7 @@
|
|||
- Preserve authentication and CSRF protections.
|
||||
- Use Linux paths and commands in examples.
|
||||
- When a live Dockerized Agent Zero target is explicitly named, verify that exact runtime instead of assuming a fixed localhost port.
|
||||
- Message-loop completion flows through a response tool with `break_loop`; plain or malformed Chat Completions text enters repair, and native Responses output text is normalized through the same response-tool path.
|
||||
- Copy live core-plugin changes back into tracked source under `plugins/`.
|
||||
- Develop new custom plugins under ignored `usr/plugins/`; tracked bundled plugins live under `plugins/`.
|
||||
- Use the framework runtime for backend and plugin-hook verification, not the separate agent execution runtime.
|
||||
|
|
|
|||
16
agent.py
16
agent.py
|
|
@ -1119,10 +1119,18 @@ class Agent:
|
|||
or extract_tools.is_misformatted_tool_request(llm_result.reasoning)
|
||||
):
|
||||
message = llm_result.reasoning
|
||||
if extract_tools.extract_tool_request(message) is None:
|
||||
if extract_tools.is_misformatted_tool_request(message):
|
||||
return await self.process_tools(message)
|
||||
return message
|
||||
if (
|
||||
llm_result.mode == "responses"
|
||||
and isinstance(message, str)
|
||||
and bool(message.strip())
|
||||
and extract_tools.extract_tool_request(message) is None
|
||||
and not extract_tools.is_misformatted_tool_request(message)
|
||||
):
|
||||
return await self._execute_tool_request(
|
||||
tool_name="response",
|
||||
tool_args={"text": message},
|
||||
message=message,
|
||||
)
|
||||
return await self.process_tools(message)
|
||||
|
||||
async def _execute_tool_request(
|
||||
|
|
|
|||
|
|
@ -738,7 +738,10 @@ class LiteLLMChatWrapper(SimpleChatModel):
|
|||
output["response_delta"]
|
||||
)
|
||||
)
|
||||
if stop_response is not None:
|
||||
if (
|
||||
stop_response is not None
|
||||
and not transport.policy.using_responses
|
||||
):
|
||||
result.response = stop_response
|
||||
break
|
||||
else:
|
||||
|
|
@ -761,7 +764,7 @@ class LiteLLMChatWrapper(SimpleChatModel):
|
|||
provider_model_key=self.model_name,
|
||||
capability=transport._capability_metadata(),
|
||||
)
|
||||
if result.output()["response_delta"]:
|
||||
if result.output()["response_delta"] and not llm_result.function_calls:
|
||||
llm_result.response = result.output()["response_delta"]
|
||||
if result.output()["reasoning_delta"]:
|
||||
llm_result.reasoning = result.output()["reasoning_delta"]
|
||||
|
|
|
|||
|
|
@ -5,7 +5,10 @@ from types import SimpleNamespace
|
|||
|
||||
import pytest
|
||||
|
||||
from helpers import files
|
||||
from agent import Agent, LoopData
|
||||
from helpers import extension, files, mcp_handler
|
||||
from helpers.llm_result import LLMResult
|
||||
from helpers.log import Log
|
||||
from plugins._goal.api.goal import Goal as GoalApi
|
||||
from plugins._goal.commands import goal_command
|
||||
from plugins._goal.tools import goal
|
||||
|
|
@ -180,3 +183,52 @@ async def test_active_goal_keeps_response_tool_running(context_id: str):
|
|||
response = await tool.execute()
|
||||
assert response.break_loop is True
|
||||
assert response.message == "Can you decide?"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_native_responses_text_uses_active_goal_response_override(
|
||||
context_id: str,
|
||||
monkeypatch,
|
||||
):
|
||||
goal.create_goal(context_id, "Keep going")
|
||||
recorded = []
|
||||
|
||||
async def no_op(*args, **kwargs):
|
||||
return None
|
||||
|
||||
class NoMcpTools:
|
||||
def get_tool(self, agent, tool_name):
|
||||
return None
|
||||
|
||||
agent = object.__new__(Agent)
|
||||
agent.context = SimpleNamespace(id=context_id, log=Log())
|
||||
agent.loop_data = LoopData()
|
||||
agent.data = {}
|
||||
agent.handle_intervention = no_op
|
||||
agent._log_response_builtin_items = no_op
|
||||
agent.hist_add_tool_result = lambda *args, **kwargs: recorded.append((args, kwargs))
|
||||
|
||||
def get_tool(name, method, args, message, loop_data, **kwargs):
|
||||
return ResponseTool(agent, name, method, args, message, loop_data)
|
||||
|
||||
agent.get_tool = get_tool
|
||||
monkeypatch.setattr(extension, "call_extensions_async", no_op)
|
||||
monkeypatch.setattr(mcp_handler.MCPConfig, "get_instance", lambda: NoMcpTools())
|
||||
|
||||
result = await Agent.process_llm_result_tools(
|
||||
agent,
|
||||
LLMResult(response="Checkpoint for the user."),
|
||||
)
|
||||
|
||||
assert result is None
|
||||
assert recorded[0][0][0] == "response"
|
||||
assert recorded[0][0][1].startswith("Goal still active.")
|
||||
assert recorded[0][1] == {}
|
||||
|
||||
goal.update_goal(context_id, status="complete")
|
||||
result = await Agent.process_llm_result_tools(
|
||||
agent,
|
||||
LLMResult(response="Finished."),
|
||||
)
|
||||
|
||||
assert result == "Finished."
|
||||
|
|
|
|||
|
|
@ -23,6 +23,41 @@ async def test_response_tool_accepts_text_or_message(args) -> None:
|
|||
assert response.break_loop is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_response_tool_uses_non_empty_legacy_message_fallback() -> None:
|
||||
tool = ResponseTool(
|
||||
None,
|
||||
"response",
|
||||
None,
|
||||
{"text": "", "message": "legacy"},
|
||||
"",
|
||||
None,
|
||||
)
|
||||
|
||||
response = await tool.execute()
|
||||
|
||||
assert response.message == "legacy"
|
||||
assert response.break_loop is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"args",
|
||||
[
|
||||
{},
|
||||
{"text": ""},
|
||||
{"text": " "},
|
||||
{"text": None},
|
||||
{"message": "\n\t"},
|
||||
],
|
||||
)
|
||||
async def test_response_tool_rejects_empty_arguments(args) -> None:
|
||||
tool = ResponseTool(None, "response", None, args, "", None)
|
||||
|
||||
with pytest.raises(RepairableException, match="non-empty top-level"):
|
||||
await tool.execute()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_response_tool_rejects_nested_response_args() -> None:
|
||||
tool = ResponseTool(
|
||||
|
|
@ -39,5 +74,5 @@ async def test_response_tool_rejects_nested_response_args() -> None:
|
|||
None,
|
||||
)
|
||||
|
||||
with pytest.raises(RepairableException, match="top-level text or message"):
|
||||
with pytest.raises(RepairableException, match="non-empty top-level"):
|
||||
await tool.execute()
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import asyncio
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -44,28 +44,6 @@ class _AsyncEventStream:
|
|||
self.closed = True
|
||||
|
||||
|
||||
class _StallingAsyncEventStream:
|
||||
def __init__(self, event: dict):
|
||||
self.event = event
|
||||
self.sent = False
|
||||
self.closed = False
|
||||
self._stalled = asyncio.Event()
|
||||
|
||||
def __aiter__(self):
|
||||
return self
|
||||
|
||||
async def __anext__(self):
|
||||
if not self.sent:
|
||||
self.sent = True
|
||||
return self.event
|
||||
await self._stalled.wait()
|
||||
raise StopAsyncIteration
|
||||
|
||||
async def aclose(self):
|
||||
self.closed = True
|
||||
self._stalled.set()
|
||||
|
||||
|
||||
def test_llm_result_persists_only_durable_responses_metadata():
|
||||
result = LLMResult.from_response(
|
||||
{
|
||||
|
|
@ -340,13 +318,58 @@ async def test_unified_turn_captures_response_id_without_stop_request(monkeypatc
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unified_turn_stops_responses_stream_after_callback_stop(monkeypatch):
|
||||
message = (
|
||||
'{"thoughts":["test"],"actions":['
|
||||
'{"tool_name":"response","tool_args":{"text":"ok"}}]}'
|
||||
)
|
||||
stream = _StallingAsyncEventStream(
|
||||
{"type": "response.output_text.delta", "delta": message}
|
||||
async def test_unified_turn_waits_for_completed_native_responses_calls(monkeypatch):
|
||||
calls = [
|
||||
{
|
||||
"type": "function_call",
|
||||
"id": "fc_1",
|
||||
"call_id": "call_1",
|
||||
"name": "lookup",
|
||||
"arguments": '{"q":"a0"}',
|
||||
},
|
||||
{
|
||||
"type": "function_call",
|
||||
"id": "fc_2",
|
||||
"call_id": "call_2",
|
||||
"name": "summarize",
|
||||
"arguments": '{"style":"short"}',
|
||||
},
|
||||
]
|
||||
stream = _AsyncEventStream(
|
||||
[
|
||||
{
|
||||
"type": "response.output_item.added",
|
||||
"output_index": 0,
|
||||
"item": {**calls[0], "arguments": ""},
|
||||
},
|
||||
{
|
||||
"type": "response.function_call_arguments.done",
|
||||
"item_id": "fc_1",
|
||||
"output_index": 0,
|
||||
"name": "lookup",
|
||||
"arguments": calls[0]["arguments"],
|
||||
},
|
||||
{
|
||||
"type": "response.output_item.added",
|
||||
"output_index": 1,
|
||||
"item": {**calls[1], "arguments": ""},
|
||||
},
|
||||
{
|
||||
"type": "response.function_call_arguments.done",
|
||||
"item_id": "fc_2",
|
||||
"output_index": 1,
|
||||
"name": "summarize",
|
||||
"arguments": calls[1]["arguments"],
|
||||
},
|
||||
{
|
||||
"type": "response.completed",
|
||||
"response": {
|
||||
"id": "resp_parallel",
|
||||
"output": calls,
|
||||
"usage": {"input_tokens": 10, "output_tokens": 5},
|
||||
},
|
||||
},
|
||||
]
|
||||
)
|
||||
|
||||
async def fake_aresponses(*args, **kwargs):
|
||||
|
|
@ -367,16 +390,26 @@ async def test_unified_turn_stops_responses_stream_after_callback_stop(monkeypat
|
|||
async def response_callback(chunk: str, full: str):
|
||||
return full if extract_tools.extract_tool_request(full) else None
|
||||
|
||||
result = await asyncio.wait_for(
|
||||
wrapper.unified_turn(
|
||||
messages=[HumanMessage(content="hi")],
|
||||
response_callback=response_callback,
|
||||
),
|
||||
timeout=1,
|
||||
result = await wrapper.unified_turn(
|
||||
messages=[HumanMessage(content="hi")],
|
||||
response_callback=response_callback,
|
||||
)
|
||||
|
||||
assert result.response == message
|
||||
assert stream.closed is True
|
||||
assert stream.index == 5
|
||||
assert stream.closed is False
|
||||
assert result.mode == "responses"
|
||||
assert result.response_id == "resp_parallel"
|
||||
assert result.usage == {"input_tokens": 10, "output_tokens": 5}
|
||||
assert [call.name for call in result.function_calls] == ["lookup", "summarize"]
|
||||
assert json.loads(result.response) == {
|
||||
"tool_name": "parallel_tool_calls",
|
||||
"tool_args": {
|
||||
"calls": [
|
||||
{"tool_name": "lookup", "tool_args": {"q": "a0"}},
|
||||
{"tool_name": "summarize", "tool_args": {"style": "short"}},
|
||||
]
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_collect_response_ids_from_agent_state_and_history_metadata():
|
||||
|
|
@ -477,47 +510,81 @@ async def test_agent_executes_native_responses_function_call_and_records_output(
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_agent_routes_only_complete_text_tool_requests() -> None:
|
||||
async def test_agent_routes_chat_retries_and_native_responses_text() -> None:
|
||||
agent = object.__new__(Agent)
|
||||
processed: list[str] = []
|
||||
executed: list[dict] = []
|
||||
|
||||
async def log_builtin_items(result):
|
||||
return None
|
||||
|
||||
async def process_tools(message):
|
||||
processed.append(message)
|
||||
return message
|
||||
return None
|
||||
|
||||
async def execute_tool_request(**kwargs):
|
||||
executed.append(kwargs)
|
||||
return None
|
||||
|
||||
agent._log_response_builtin_items = log_builtin_items
|
||||
agent.process_tools = process_tools
|
||||
agent._execute_tool_request = execute_tool_request
|
||||
|
||||
tool_request = '{"type":"function","name":"response","parameters":{"text":"ok"}}'
|
||||
for message in (
|
||||
chat_messages = (
|
||||
"Plain final answer.",
|
||||
'{"status":"planning"}',
|
||||
f"Example tool JSON: {tool_request}",
|
||||
):
|
||||
f"∂\n{tool_request}",
|
||||
(
|
||||
'{"thoughts":["Done"],"headline":"Done","tool_args":'
|
||||
'{"text":"ok","tool_name":"response"}'
|
||||
),
|
||||
)
|
||||
for message in chat_messages:
|
||||
assert await Agent.process_llm_result_tools(
|
||||
agent, LLMResult.from_chat(response=message)
|
||||
) == message
|
||||
assert processed == []
|
||||
) is None
|
||||
assert processed == list(chat_messages)
|
||||
|
||||
processed.clear()
|
||||
responses_messages = (
|
||||
"Plain final answer.",
|
||||
'{"status":"planning"}',
|
||||
f"Example tool JSON: {tool_request}",
|
||||
)
|
||||
for message in responses_messages:
|
||||
assert await Agent.process_llm_result_tools(
|
||||
agent, LLMResult(response=message)
|
||||
) is None
|
||||
assert processed == []
|
||||
assert executed == [
|
||||
{
|
||||
"tool_name": "response",
|
||||
"tool_args": {"text": message},
|
||||
"message": message,
|
||||
}
|
||||
for message in responses_messages
|
||||
]
|
||||
|
||||
processed.clear()
|
||||
executed.clear()
|
||||
assert await Agent.process_llm_result_tools(
|
||||
agent, LLMResult.from_chat(response=tool_request)
|
||||
) == tool_request
|
||||
) is None
|
||||
assert processed == [tool_request]
|
||||
|
||||
processed.clear()
|
||||
assert await Agent.process_llm_result_tools(
|
||||
agent, LLMResult(response="", reasoning=tool_request)
|
||||
) == tool_request
|
||||
) is None
|
||||
assert processed == [tool_request]
|
||||
|
||||
processed.clear()
|
||||
assert await Agent.process_llm_result_tools(
|
||||
agent, LLMResult(response="", reasoning='{"status":"planning"}')
|
||||
) == ""
|
||||
assert processed == []
|
||||
) is None
|
||||
assert processed == [""]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -235,25 +235,22 @@ def test_provider_defaults_do_not_freeze_litellm_global_kwargs(monkeypatch):
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unified_call_stops_after_canonical_root_snapshot(monkeypatch):
|
||||
async def test_unified_call_stops_chat_after_canonical_root_snapshot(monkeypatch):
|
||||
stream = _AsyncChunkStream(
|
||||
[
|
||||
{"type": "response.created"},
|
||||
_response_event('{"tool_name":"response","tool_args":{"text":"hello"}}'),
|
||||
_response_event(" unreachable"),
|
||||
_chunk('{"tool_name":"response","tool_args":{"text":"hello"}}'),
|
||||
_chunk(" unreachable"),
|
||||
]
|
||||
)
|
||||
|
||||
async def fake_aresponses(*args, **kwargs):
|
||||
async def fake_acompletion(*args, **kwargs):
|
||||
assert kwargs["stream"] is True
|
||||
assert kwargs["input"] == ""
|
||||
assert kwargs["store"] is True
|
||||
return stream
|
||||
|
||||
async def fake_rate_limiter(*args, **kwargs):
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(litellm_transport, "aresponses", fake_aresponses)
|
||||
monkeypatch.setattr(litellm_transport, "acompletion", fake_acompletion)
|
||||
monkeypatch.setattr(models, "apply_rate_limiter", fake_rate_limiter)
|
||||
monkeypatch.setattr(
|
||||
models.settings,
|
||||
|
|
@ -265,6 +262,7 @@ async def test_unified_call_stops_after_canonical_root_snapshot(monkeypatch):
|
|||
model="test-model",
|
||||
provider="openai",
|
||||
model_config=None,
|
||||
a0_api_mode="chat",
|
||||
)
|
||||
|
||||
seen: list[tuple[str, str]] = []
|
||||
|
|
@ -280,7 +278,7 @@ async def test_unified_call_stops_after_canonical_root_snapshot(monkeypatch):
|
|||
|
||||
assert response == '{"tool_name":"response","tool_args":{"text":"hello"}}'
|
||||
assert reasoning == ""
|
||||
assert stream.index == 2
|
||||
assert stream.index == 1
|
||||
assert stream.closed is True
|
||||
assert len(seen) == 1
|
||||
assert seen[0][1] == '{"tool_name":"response","tool_args":{"text":"hello"}}'
|
||||
|
|
@ -290,25 +288,22 @@ async def test_unified_call_stops_after_canonical_root_snapshot(monkeypatch):
|
|||
async def test_unified_call_does_not_stop_for_embedded_tool_json(monkeypatch):
|
||||
stream = _AsyncChunkStream(
|
||||
[
|
||||
{"type": "response.created"},
|
||||
_response_event('Preamble {"note":"not the tool"}.\n'),
|
||||
_response_event(
|
||||
_chunk('Preamble {"note":"not the tool"}.\n'),
|
||||
_chunk(
|
||||
'{"tool_name":"response","tool_args":{"text":"ok"}} trailing text'
|
||||
),
|
||||
_response_event(" unreachable"),
|
||||
_chunk(" unreachable"),
|
||||
]
|
||||
)
|
||||
|
||||
async def fake_aresponses(*args, **kwargs):
|
||||
async def fake_acompletion(*args, **kwargs):
|
||||
assert kwargs["stream"] is True
|
||||
assert kwargs["input"] == ""
|
||||
assert kwargs["store"] is True
|
||||
return stream
|
||||
|
||||
async def fake_rate_limiter(*args, **kwargs):
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(litellm_transport, "aresponses", fake_aresponses)
|
||||
monkeypatch.setattr(litellm_transport, "acompletion", fake_acompletion)
|
||||
monkeypatch.setattr(models, "apply_rate_limiter", fake_rate_limiter)
|
||||
monkeypatch.setattr(
|
||||
models.settings,
|
||||
|
|
@ -320,6 +315,7 @@ async def test_unified_call_does_not_stop_for_embedded_tool_json(monkeypatch):
|
|||
model="test-model",
|
||||
provider="openai",
|
||||
model_config=None,
|
||||
a0_api_mode="chat",
|
||||
)
|
||||
|
||||
seen: list[tuple[str, str]] = []
|
||||
|
|
@ -338,7 +334,7 @@ async def test_unified_call_does_not_stop_for_embedded_tool_json(monkeypatch):
|
|||
'{"tool_name":"response","tool_args":{"text":"ok"}} trailing text unreachable'
|
||||
)
|
||||
assert reasoning == ""
|
||||
assert stream.index == 4
|
||||
assert stream.index == 3
|
||||
assert stream.closed is False
|
||||
assert len(seen) == 3
|
||||
assert seen[0][1] == 'Preamble {"note":"not the tool"}.\n'
|
||||
|
|
|
|||
|
|
@ -7,10 +7,10 @@ class ResponseTool(Tool):
|
|||
async def execute(self, **kwargs):
|
||||
for key in ("text", "message"):
|
||||
message = self.args.get(key)
|
||||
if isinstance(message, str):
|
||||
if isinstance(message, str) and message.strip():
|
||||
return Response(message=message, break_loop=True)
|
||||
raise RepairableException(
|
||||
"response tool requires a top-level text or message string argument"
|
||||
"response tool requires a non-empty top-level text or message string argument"
|
||||
)
|
||||
|
||||
async def before_execution(self, **kwargs):
|
||||
|
|
|
|||
|
|
@ -22,7 +22,8 @@
|
|||
- Update this file whenever tool arguments, output shape, `break_loop` behavior, intervention handling, prompt instructions, or side effects change.
|
||||
- `ResponseTool` is a `Tool`.
|
||||
- `ResponseTool` defines `execute(...)`.
|
||||
- `ResponseTool` requires a top-level string `text` or legacy `message` argument.
|
||||
- `ResponseTool` requires a non-empty top-level string `text` or legacy `message`
|
||||
argument, preferring `text` and falling back to `message` when `text` is blank.
|
||||
Invalid arguments raise `RepairableException` so the agent can surface a correction
|
||||
warning and retry rather than crash.
|
||||
- Imported dependency areas include: `helpers.errors`, `helpers.tool`.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue