refactor(tools): split wait_for_message into respond_to_user + wait_for_agents

One tool was doing three jobs (wait on the user, wait on other agents, and
- wrongly - wait for a long-running command), so the driver had to guess which
one an agent meant and used parent_id as the proxy: the root waits for a human,
everyone else waits for agents. That proxy is wrong, since the user can message
any agent from the TUI's agent tree.

Tool identity now carries the intent, and the coordinator records it as a
wait_kind that survives snapshot/restore:

  respond_to_user  -> wait_kind="user",   never auto-resumed (root or not)
  wait_for_agents  -> wait_kind="agents", auto-resumed on a 300s timer
  recovery exhaust -> wait_kind="stalled"

respond_to_user fuses the message and the yield into one call, so there is no
way to answer and then forget to stop - the two-step that gpt-4o-mini skipped
2/2 in live testing. Plain text still renders as before.

Auto-resume is also bounded now: an agent that re-parks after every timeout
burned a model turn every 300s for the rest of the scan (and, since parked
children notify their parent, spammed the parent's inbox on the same cycle).
After _MAX_IDLE_AUTO_RESUMES it stays parked until a real message arrives.
This commit is contained in:
Ahmed Allam
2026-08-01 22:13:07 +00:00
parent b7bf52c468
commit 8b464dae5d
20 changed files with 539 additions and 128 deletions
+27 -1
View File
@@ -2,18 +2,29 @@
from __future__ import annotations
from typing import TYPE_CHECKING, Any
import pytest
from agents.tool import FunctionTool
from strix.agents import factory
if TYPE_CHECKING:
from agents.tool_context import ToolContext
def _tool(name: str) -> FunctionTool:
# A per-tool closure keeps two same-named tools unequal, which is what the
# duplicate-name tests exercise.
async def invoke(_ctx: ToolContext[Any], _input: str) -> str:
return "ok"
return FunctionTool(
name=name,
description="test tool",
params_json_schema={"type": "object", "properties": {}, "additionalProperties": False},
on_invoke_tool=lambda _ctx, _inp: "ok",
on_invoke_tool=invoke,
)
@@ -86,3 +97,18 @@ def test_no_override_renders_builtin_prompt() -> None:
assert isinstance(agent.instructions, str)
assert agent.instructions != ""
def test_respond_to_user_is_interactive_only() -> None:
"""Yielding to the user is meaningless when no user is attached."""
interactive = factory.build_strix_agent(is_root=True, interactive=True)
autonomous = factory.build_strix_agent(is_root=True, interactive=False)
assert "respond_to_user" in [t.name for t in interactive.tools]
assert "respond_to_user" not in [t.name for t in autonomous.tools]
def test_wait_for_agents_is_available_in_both_modes() -> None:
for interactive in (True, False):
agent = factory.build_strix_agent(is_root=True, interactive=interactive)
assert "wait_for_agents" in [t.name for t in agent.tools]
+25 -10
View File
@@ -7,7 +7,7 @@ from unittest.mock import MagicMock, patch
import pytest
from strix.core import execution
from strix.core.agents import AgentCoordinator
from strix.core.agents import AgentCoordinator, WaitKind
from strix.core.execution import _start_child_runner, run_agent_loop
from strix.core.hooks import BudgetExceededError, ReportUsageHooks
from strix.core.sessions import open_agent_session
@@ -42,23 +42,32 @@ class _FakeStream:
hooks: ReportUsageHooks,
context: dict[str, Any],
agent: Any,
coordinator: AgentCoordinator,
) -> None:
self._ledger = ledger
self._hooks = hooks
self._context = context
self._agent = agent
self._coordinator = coordinator
self.run_loop_exception: BaseException | None = None
self.final_output = None
async def stream_events(self) -> AsyncIterator[Any]:
agent_id = str(self._context.get("agent_id"))
self._ledger.cost += COST_PER_CALL
self._ledger.calls.append(str(self._context.get("agent_id")))
self._ledger.calls.append(agent_id)
ctx_wrapper = MagicMock()
ctx_wrapper.context = self._context
try:
await self._hooks.on_llm_end(ctx_wrapper, self._agent, MagicMock())
except Exception as exc: # noqa: BLE001
self.run_loop_exception = exc
# Stand in for the explicit yield tool a real turn ends with. Without it
# every turn looks like a forgotten tool call and burns the recovery
# budget, which is a different scenario from the one under test here.
if self._coordinator.statuses.get(agent_id) == "running":
wait_kind: WaitKind = "user" if self._context.get("parent_id") is None else "agents"
await self._coordinator.park_waiting(agent_id, wait_kind=wait_kind)
items: tuple[Any, ...] = ()
for item in items:
yield item
@@ -67,7 +76,7 @@ class _FakeStream:
return
def _fake_runner(ledger: _FakeLedger) -> Any:
def _fake_runner(ledger: _FakeLedger, coordinator: AgentCoordinator) -> Any:
class _FakeRunner:
@staticmethod
def run_streamed(
@@ -80,7 +89,13 @@ def _fake_runner(ledger: _FakeLedger) -> Any:
session: Any, # noqa: ARG004
hooks: ReportUsageHooks,
) -> _FakeStream:
return _FakeStream(ledger=ledger, hooks=hooks, context=context, agent=agent)
return _FakeStream(
ledger=ledger,
hooks=hooks,
context=context,
agent=agent,
coordinator=coordinator,
)
return _FakeRunner
@@ -103,10 +118,10 @@ async def test_full_budget_lifecycle_reserve_then_cap( # noqa: PLR0915
) -> None:
ledger = _FakeLedger()
hooks = ReportUsageHooks(model="test-model", max_budget_usd=MAX_BUDGET)
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger))
coordinator = AgentCoordinator()
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger, coordinator))
monkeypatch.setattr(execution, "_compact_session", _noop_compact)
coordinator = AgentCoordinator()
db_path = tmp_path / "agents.sqlite"
sessions: list[Any] = []
run_config = MagicMock()
@@ -218,7 +233,6 @@ async def test_respawned_children_after_reserve_never_spend(
ledger = _FakeLedger()
ledger.cost = 9.5
hooks = ReportUsageHooks(model="test-model", max_budget_usd=MAX_BUDGET)
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger))
monkeypatch.setattr(execution, "_compact_session", _noop_compact)
coordinator = AgentCoordinator()
@@ -230,6 +244,7 @@ async def test_respawned_children_after_reserve_never_spend(
restored = AgentCoordinator()
await restored.restore(snap)
assert restored.reserve_stopped is True
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger, restored))
sessions: list[Any] = []
with patch("strix.core.hooks.get_global_report_state", return_value=ledger):
@@ -264,7 +279,6 @@ async def test_resumed_parked_root_after_reserve_is_renotified_and_finalizes(
ledger = _FakeLedger()
ledger.cost = 9.0
hooks = ReportUsageHooks(model="test-model", max_budget_usd=MAX_BUDGET)
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger))
monkeypatch.setattr(execution, "_compact_session", _noop_compact)
coordinator = AgentCoordinator()
@@ -276,6 +290,7 @@ async def test_resumed_parked_root_after_reserve_is_renotified_and_finalizes(
restored = AgentCoordinator()
await restored.restore(snap)
assert restored.reserve_stopped is True
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger, restored))
root_session = open_agent_session("root", tmp_path / "agents.sqlite")
with patch("strix.core.hooks.get_global_report_state", return_value=ledger):
@@ -312,10 +327,10 @@ async def test_interactive_budget_pause_then_user_message_extends_and_resumes(
ledger = _FakeLedger()
ledger.cost = 9.0
hooks = ReportUsageHooks(model="test-model", max_budget_usd=MAX_BUDGET, interactive=True)
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger))
coordinator = AgentCoordinator()
monkeypatch.setattr(execution, "Runner", _fake_runner(ledger, coordinator))
monkeypatch.setattr(execution, "_compact_session", _noop_compact)
coordinator = AgentCoordinator()
coordinator.set_budget_extender(hooks.extend_budget)
await coordinator.register("root", "strix", parent_id=None)
root_session = open_agent_session("root", tmp_path / "agents.sqlite")
+60 -5
View File
@@ -847,15 +847,15 @@ async def test_interactive_text_only_turn_is_nudged_instead_of_parking(
# The retry carries an explicit "call a tool" nudge rather than empty input.
nudge = calls[1][0]["content"]
assert "without a tool call" in nudge
assert "wait_for_message" in nudge
assert "respond_to_user" in nudge
assert coordinator.statuses["root"] == "completed"
@pytest.mark.asyncio
async def test_interactive_wait_for_message_parks_without_a_nudge(
async def test_interactive_explicit_park_gets_no_nudge(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""``waiting`` is now only reachable by an explicit wait_for_message call."""
"""``waiting`` is only reachable via respond_to_user / wait_for_agents."""
coordinator = AgentCoordinator()
await coordinator.register("root", "strix", parent_id=None)
calls: list[Any] = []
@@ -898,7 +898,7 @@ async def test_interactive_subagent_exhaustion_tells_its_parent(
"""A parked child must report up so its parent stops waiting on it.
The parent is an agent, not a watching human, so a parent blocked in
wait_for_message otherwise burns its whole timeout on a completion
wait_for_agents otherwise burns its whole timeout on a completion
report the child can no longer send.
"""
coordinator = AgentCoordinator()
@@ -978,7 +978,7 @@ async def test_tool_required_message_is_persisted_to_the_session(tmp_path: Any)
stored = [cast("dict[str, Any]", i) for i in await session.get_items()]
assert "finish_scan" in stored[0]["content"]
assert "wait_for_message" in stored[0]["content"]
assert "respond_to_user" in stored[0]["content"]
session.close()
@@ -1033,3 +1033,58 @@ async def test_recovery_count_is_cleared_by_a_lifecycle_tool(
await _drive(coordinator, "root", interactive=True)
assert "root" not in coordinator.recovery_counts
@pytest.mark.asyncio
async def test_agent_awaiting_a_human_is_never_auto_resumed() -> None:
"""The user can message any agent, so parking for one is not root-only."""
coordinator = AgentCoordinator()
await coordinator.register("root", "strix", parent_id=None)
await coordinator.register("child", "recon", parent_id="root")
for agent_id in ("root", "child"):
await coordinator.park_waiting(agent_id, wait_kind="user")
assert await execution._plain_waiting_timeout(coordinator, agent_id) is None
@pytest.mark.asyncio
async def test_agent_awaiting_other_agents_is_re_checked_on_a_timer() -> None:
coordinator = AgentCoordinator()
await coordinator.register("root", "strix", parent_id=None)
await coordinator.park_waiting("root", wait_kind="agents")
timeout = await execution._plain_waiting_timeout(coordinator, "root")
assert timeout == execution._WAITING_AUTO_RESUME_TIMEOUT_S
@pytest.mark.asyncio
async def test_idle_auto_resumes_stop_after_their_budget() -> None:
"""A wedged agent must not burn a model turn per timeout for the whole scan."""
coordinator = AgentCoordinator()
await coordinator.register("child", "recon", parent_id="root")
await coordinator.park_waiting("child", wait_kind="agents")
for _ in range(execution._MAX_IDLE_AUTO_RESUMES):
assert await execution._plain_waiting_timeout(coordinator, "child") is not None
await coordinator.record_idle_resume("child")
assert await execution._plain_waiting_timeout(coordinator, "child") is None
# A real message is real progress, so the budget starts over.
await coordinator.reset_idle_resumes("child")
assert await execution._plain_waiting_timeout(coordinator, "child") is not None
@pytest.mark.asyncio
async def test_wait_kind_survives_a_snapshot_round_trip() -> None:
coordinator = AgentCoordinator()
await coordinator.register("root", "strix", parent_id=None)
await coordinator.park_waiting("root", wait_kind="user")
await coordinator.record_idle_resume("root")
restored = AgentCoordinator()
await restored.restore(await coordinator.snapshot())
assert restored.wait_kinds["root"] == "user"
assert restored.idle_resume_counts["root"] == 1
assert await execution._plain_waiting_timeout(restored, "root") is None
+66
View File
@@ -0,0 +1,66 @@
"""Tests for the ``respond_to_user`` yield tool."""
from __future__ import annotations
import json
from typing import Any
import pytest
from agents.tool_context import ToolContext
from strix.core.agents import AgentCoordinator
from strix.tools.respond.tool import respond_to_user
async def _call(context: dict[str, Any], message: str = "here is what I found") -> dict[str, Any]:
ctx = ToolContext(
context=context,
tool_name="respond_to_user",
tool_call_id="call-1",
tool_arguments="{}",
)
raw = await respond_to_user.on_invoke_tool(ctx, json.dumps({"message": message}))
return json.loads(raw) # type: ignore[no-any-return]
async def _context(*, interactive: bool, agent_id: str = "root") -> dict[str, Any]:
coordinator = AgentCoordinator()
await coordinator.register("root", "strix", parent_id=None)
return {"coordinator": coordinator, "agent_id": agent_id, "interactive": interactive}
@pytest.mark.asyncio
async def test_parks_the_agent_and_carries_the_message() -> None:
context = await _context(interactive=True)
result = await _call(context)
coordinator = context["coordinator"]
assert result["success"] is True
assert result["wait_outcome"] == "waiting"
assert result["message"] == "here is what I found"
assert coordinator.statuses["root"] == "waiting"
# Recorded as a human wait, so the driver never auto-resumes it.
assert coordinator.wait_kinds["root"] == "user"
@pytest.mark.asyncio
async def test_rejected_in_an_autonomous_run() -> None:
context = await _context(interactive=False)
result = await _call(context)
assert result["success"] is False
assert "finish_scan" in result["error"]
assert context["coordinator"].statuses["root"] == "running"
@pytest.mark.asyncio
async def test_a_message_that_already_arrived_is_taken_instead_of_parking() -> None:
context = await _context(interactive=True)
coordinator = context["coordinator"]
await coordinator.send("root", {"from": "user", "content": "wait, one more thing"})
result = await _call(context)
assert result["wait_outcome"] == "message_arrived"
assert result["pending_messages"] == 1
assert coordinator.statuses["root"] == "running"