mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
feat(delegate): orchestrator role and configurable spawn depth (default flat)
Adds role='leaf'|'orchestrator' to delegate_task. With max_spawn_depth>=2, an orchestrator child retains the 'delegation' toolset and can spawn its own workers; leaf children cannot delegate further (identical to today). Default posture is flat — max_spawn_depth=1 means a depth-0 parent's children land at the depth-1 floor and orchestrator role silently degrades to leaf. Users opt into nested delegation by raising max_spawn_depth to 2 or 3 in config.yaml. Also threads acp_command/acp_args through the main agent loop's delegate dispatch (previously silently dropped in the schema) via a new _dispatch_delegate_task helper, and adds a DelegateEvent enum with legacy-string back-compat for gateway/ACP/CLI progress consumers. Config (hermes_cli/config.py defaults): delegation.max_concurrent_children: 3 # floor-only, no upper cap delegation.max_spawn_depth: 1 # 1=flat (default), 2-3 unlock nested delegation.orchestrator_enabled: true # global kill switch Salvaged from @pefontana's PR #11215. Overrides vs. the original PR: concurrency stays at 3 (PR bumped to 5 + cap 8 — we keep the floor only, no hard ceiling); max_spawn_depth defaults to 1 (PR defaulted to 2 which silently enabled one level of orchestration for every user). Co-authored-by: pefontana <fontana.pedro93@gmail.com>
This commit is contained in:
parent
e7f8a5fea3
commit
48ecb98f8a
11 changed files with 1003 additions and 64 deletions
|
|
@ -20,11 +20,14 @@ from unittest.mock import MagicMock, patch
|
|||
from tools.delegate_tool import (
|
||||
DELEGATE_BLOCKED_TOOLS,
|
||||
DELEGATE_TASK_SCHEMA,
|
||||
DelegateEvent,
|
||||
_get_max_concurrent_children,
|
||||
_LEGACY_EVENT_MAP,
|
||||
MAX_DEPTH,
|
||||
check_delegate_requirements,
|
||||
delegate_task,
|
||||
_build_child_agent,
|
||||
_build_child_progress_callback,
|
||||
_build_child_system_prompt,
|
||||
_strip_blocked_tools,
|
||||
_resolve_child_credential_pool,
|
||||
|
|
@ -568,8 +571,16 @@ class TestBlockedTools(unittest.TestCase):
|
|||
self.assertIn(tool, DELEGATE_BLOCKED_TOOLS)
|
||||
|
||||
def test_constants(self):
|
||||
from tools.delegate_tool import (
|
||||
_get_max_spawn_depth, _get_orchestrator_enabled,
|
||||
_MIN_SPAWN_DEPTH, _MAX_SPAWN_DEPTH_CAP,
|
||||
)
|
||||
self.assertEqual(_get_max_concurrent_children(), 3)
|
||||
self.assertEqual(MAX_DEPTH, 2)
|
||||
self.assertEqual(MAX_DEPTH, 1)
|
||||
self.assertEqual(_get_max_spawn_depth(), 1) # default: flat
|
||||
self.assertTrue(_get_orchestrator_enabled()) # default
|
||||
self.assertEqual(_MIN_SPAWN_DEPTH, 1)
|
||||
self.assertEqual(_MAX_SPAWN_DEPTH_CAP, 3)
|
||||
|
||||
|
||||
class TestDelegationCredentialResolution(unittest.TestCase):
|
||||
|
|
@ -1325,5 +1336,635 @@ class TestDelegationReasoningEffort(unittest.TestCase):
|
|||
self.assertEqual(call_kwargs["reasoning_config"], {"enabled": True, "effort": "medium"})
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Dispatch helper, progress events, concurrency
|
||||
# =========================================================================
|
||||
|
||||
class TestDispatchDelegateTask(unittest.TestCase):
|
||||
"""Tests for the _dispatch_delegate_task helper and full param forwarding."""
|
||||
|
||||
@patch("tools.delegate_tool._load_config", return_value={})
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
def test_acp_args_forwarded(self, mock_creds, mock_cfg):
|
||||
"""Both acp_command and acp_args reach delegate_task via the helper."""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
with patch("tools.delegate_tool._build_child_agent") as mock_build:
|
||||
mock_child = MagicMock()
|
||||
mock_child.run_conversation.return_value = {
|
||||
"final_response": "done", "completed": True,
|
||||
"api_calls": 1, "messages": [],
|
||||
}
|
||||
mock_child._delegate_saved_tool_names = []
|
||||
mock_child._credential_pool = None
|
||||
mock_child.session_prompt_tokens = 0
|
||||
mock_child.session_completion_tokens = 0
|
||||
mock_child.model = "test"
|
||||
mock_build.return_value = mock_child
|
||||
|
||||
delegate_task(
|
||||
goal="test",
|
||||
acp_command="claude",
|
||||
acp_args=["--acp", "--stdio"],
|
||||
parent_agent=parent,
|
||||
)
|
||||
_, kwargs = mock_build.call_args
|
||||
self.assertEqual(kwargs["override_acp_command"], "claude")
|
||||
self.assertEqual(kwargs["override_acp_args"], ["--acp", "--stdio"])
|
||||
|
||||
class TestDelegateEventEnum(unittest.TestCase):
|
||||
"""Tests for DelegateEvent enum and back-compat aliases."""
|
||||
|
||||
def test_enum_values_are_strings(self):
|
||||
for event in DelegateEvent:
|
||||
self.assertIsInstance(event.value, str)
|
||||
self.assertTrue(event.value.startswith("delegate."))
|
||||
|
||||
def test_legacy_map_covers_all_old_names(self):
|
||||
expected_legacy = {"_thinking", "reasoning.available",
|
||||
"tool.started", "tool.completed", "subagent_progress"}
|
||||
self.assertEqual(set(_LEGACY_EVENT_MAP.keys()), expected_legacy)
|
||||
|
||||
def test_legacy_map_values_are_delegate_events(self):
|
||||
for old_name, event in _LEGACY_EVENT_MAP.items():
|
||||
self.assertIsInstance(event, DelegateEvent)
|
||||
|
||||
def test_progress_callback_normalises_tool_started(self):
|
||||
"""_build_child_progress_callback handles tool.started via enum."""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
parent.tool_progress_callback = MagicMock()
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
self.assertIsNotNone(cb)
|
||||
|
||||
cb("tool.started", tool_name="terminal", preview="ls")
|
||||
parent._delegate_spinner.print_above.assert_called()
|
||||
|
||||
def test_progress_callback_normalises_thinking(self):
|
||||
"""Both _thinking and reasoning.available route to TASK_THINKING."""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
parent.tool_progress_callback = None
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
|
||||
cb("_thinking", tool_name=None, preview="pondering...")
|
||||
assert any("💭" in str(c) for c in parent._delegate_spinner.print_above.call_args_list)
|
||||
|
||||
parent._delegate_spinner.print_above.reset_mock()
|
||||
cb("reasoning.available", tool_name=None, preview="hmm")
|
||||
assert any("💭" in str(c) for c in parent._delegate_spinner.print_above.call_args_list)
|
||||
|
||||
def test_progress_callback_tool_completed_is_noop(self):
|
||||
"""tool.completed is normalised but produces no display output."""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
parent.tool_progress_callback = None
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
cb("tool.completed", tool_name="terminal")
|
||||
parent._delegate_spinner.print_above.assert_not_called()
|
||||
|
||||
def test_progress_callback_ignores_unknown_events(self):
|
||||
"""Unknown event types are silently ignored."""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
# Should not raise
|
||||
cb("some.unknown.event", tool_name="x")
|
||||
parent._delegate_spinner.print_above.assert_not_called()
|
||||
|
||||
def test_progress_callback_accepts_enum_value_directly(self):
|
||||
"""cb(DelegateEvent.TASK_THINKING, ...) must route to the thinking
|
||||
branch. Pre-fix the callback only handled legacy strings via
|
||||
_LEGACY_EVENT_MAP.get and silently dropped enum-typed callers."""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
parent.tool_progress_callback = None
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
cb(DelegateEvent.TASK_THINKING, preview="pondering")
|
||||
# If the enum was accepted, the thinking emoji got printed.
|
||||
assert any(
|
||||
"💭" in str(c)
|
||||
for c in parent._delegate_spinner.print_above.call_args_list
|
||||
)
|
||||
|
||||
def test_progress_callback_accepts_new_style_string(self):
|
||||
"""cb('delegate.task_thinking', ...) — the string form of the
|
||||
enum value — must route to the thinking branch too, so new-style
|
||||
emitters don't have to import DelegateEvent."""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
cb("delegate.task_thinking", preview="hmm")
|
||||
assert any(
|
||||
"💭" in str(c)
|
||||
for c in parent._delegate_spinner.print_above.call_args_list
|
||||
)
|
||||
|
||||
def test_progress_callback_task_progress_not_misrendered(self):
|
||||
"""'subagent_progress' (legacy name for TASK_PROGRESS) carries a
|
||||
pre-batched summary in the tool_name slot. Before the fix, this
|
||||
fell through to the TASK_TOOL_STARTED rendering path, treating
|
||||
the summary string as a tool name. After the fix: distinct
|
||||
render (no tool-start emoji lookup) and pass-through relay
|
||||
upward (no re-batching).
|
||||
|
||||
Regression path only reachable once nested orchestration is
|
||||
enabled: nested orchestrators relay subagent_progress from
|
||||
grandchildren upward through this callback.
|
||||
"""
|
||||
parent = _make_mock_parent()
|
||||
parent._delegate_spinner = MagicMock()
|
||||
parent.tool_progress_callback = MagicMock()
|
||||
|
||||
cb = _build_child_progress_callback(0, "test goal", parent, task_count=1)
|
||||
cb("subagent_progress", tool_name="🔀 [1] terminal, file")
|
||||
|
||||
# Spinner gets a distinct 🔀-prefixed line, NOT a tool emoji
|
||||
# followed by the summary string as if it were a tool name.
|
||||
calls = parent._delegate_spinner.print_above.call_args_list
|
||||
self.assertTrue(any("🔀 🔀 [1] terminal, file" in str(c) for c in calls))
|
||||
# Parent callback receives the relay (pass-through, no re-batching).
|
||||
parent.tool_progress_callback.assert_called_once()
|
||||
# No '⚡' tool-start emoji should appear — that's the pre-fix bug.
|
||||
self.assertFalse(any("⚡" in str(c) for c in calls))
|
||||
|
||||
|
||||
class TestConcurrencyDefaults(unittest.TestCase):
|
||||
"""Tests for the concurrency default and no hard ceiling."""
|
||||
|
||||
@patch("tools.delegate_tool._load_config", return_value={})
|
||||
def test_default_is_three(self, mock_cfg):
|
||||
# Clear env var if set
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
self.assertEqual(_get_max_concurrent_children(), 3)
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_concurrent_children": 10})
|
||||
def test_no_upper_ceiling(self, mock_cfg):
|
||||
"""Users can raise concurrency as high as they want — no hard cap."""
|
||||
self.assertEqual(_get_max_concurrent_children(), 10)
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_concurrent_children": 100})
|
||||
def test_very_high_values_honored(self, mock_cfg):
|
||||
self.assertEqual(_get_max_concurrent_children(), 100)
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_concurrent_children": 0})
|
||||
def test_zero_clamped_to_one(self, mock_cfg):
|
||||
"""Floor of 1 is enforced; zero or negative values raise to 1."""
|
||||
self.assertEqual(_get_max_concurrent_children(), 1)
|
||||
|
||||
@patch("tools.delegate_tool._load_config", return_value={})
|
||||
def test_env_var_honored_uncapped(self, mock_cfg):
|
||||
with patch.dict(os.environ, {"DELEGATION_MAX_CONCURRENT_CHILDREN": "12"}):
|
||||
self.assertEqual(_get_max_concurrent_children(), 12)
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_concurrent_children": 6})
|
||||
def test_configured_value_returned(self, mock_cfg):
|
||||
self.assertEqual(_get_max_concurrent_children(), 6)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# max_spawn_depth clamping
|
||||
# =========================================================================
|
||||
|
||||
class TestMaxSpawnDepth(unittest.TestCase):
|
||||
"""Tests for _get_max_spawn_depth clamping and fallback behavior."""
|
||||
|
||||
@patch("tools.delegate_tool._load_config", return_value={})
|
||||
def test_max_spawn_depth_defaults_to_1(self, mock_cfg):
|
||||
from tools.delegate_tool import _get_max_spawn_depth
|
||||
self.assertEqual(_get_max_spawn_depth(), 1)
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 0})
|
||||
def test_max_spawn_depth_clamped_below_one(self, mock_cfg):
|
||||
import logging
|
||||
from tools.delegate_tool import _get_max_spawn_depth
|
||||
with self.assertLogs("tools.delegate_tool", level=logging.WARNING) as cm:
|
||||
result = _get_max_spawn_depth()
|
||||
self.assertEqual(result, 1)
|
||||
self.assertTrue(any("clamping to 1" in m for m in cm.output))
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 99})
|
||||
def test_max_spawn_depth_clamped_above_three(self, mock_cfg):
|
||||
import logging
|
||||
from tools.delegate_tool import _get_max_spawn_depth
|
||||
with self.assertLogs("tools.delegate_tool", level=logging.WARNING) as cm:
|
||||
result = _get_max_spawn_depth()
|
||||
self.assertEqual(result, 3)
|
||||
self.assertTrue(any("clamping to 3" in m for m in cm.output))
|
||||
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": "not-a-number"})
|
||||
def test_max_spawn_depth_invalid_falls_back_to_default(self, mock_cfg):
|
||||
from tools.delegate_tool import _get_max_spawn_depth
|
||||
self.assertEqual(_get_max_spawn_depth(), 1)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# role param plumbing
|
||||
# =========================================================================
|
||||
#
|
||||
# These tests cover the schema + signature + stash plumbing of the role
|
||||
# param. The full role-honoring behavior (toolset re-add, role-aware
|
||||
# prompt) lives in TestOrchestratorRoleBehavior below; these tests only
|
||||
# assert on _delegate_role stashing and on the schema shape.
|
||||
|
||||
|
||||
class TestOrchestratorRoleSchema(unittest.TestCase):
|
||||
"""Tests that the role param reaches the child via dispatch."""
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 2})
|
||||
def _run_with_mock_child(self, role_arg, mock_cfg, mock_creds):
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = MagicMock()
|
||||
mock_child.run_conversation.return_value = {
|
||||
"final_response": "done", "completed": True,
|
||||
"api_calls": 1, "messages": [],
|
||||
}
|
||||
mock_child._delegate_saved_tool_names = []
|
||||
mock_child._credential_pool = None
|
||||
mock_child.session_prompt_tokens = 0
|
||||
mock_child.session_completion_tokens = 0
|
||||
mock_child.model = "test"
|
||||
MockAgent.return_value = mock_child
|
||||
kwargs = {"goal": "test", "parent_agent": parent}
|
||||
if role_arg is not _SENTINEL:
|
||||
kwargs["role"] = role_arg
|
||||
delegate_task(**kwargs)
|
||||
return mock_child
|
||||
|
||||
def test_default_role_is_leaf(self):
|
||||
child = self._run_with_mock_child(_SENTINEL)
|
||||
self.assertEqual(child._delegate_role, "leaf")
|
||||
|
||||
def test_explicit_orchestrator_role_stashed(self):
|
||||
"""role='orchestrator' reaches _build_child_agent and is stashed.
|
||||
Full behavior (toolset re-add) lands in commit 3; commit 2 only
|
||||
verifies the plumbing."""
|
||||
child = self._run_with_mock_child("orchestrator")
|
||||
self.assertEqual(child._delegate_role, "orchestrator")
|
||||
|
||||
def test_unknown_role_coerces_to_leaf(self):
|
||||
"""role='nonsense' → _normalize_role warns and returns 'leaf'."""
|
||||
import logging
|
||||
with self.assertLogs("tools.delegate_tool", level=logging.WARNING) as cm:
|
||||
child = self._run_with_mock_child("nonsense")
|
||||
self.assertEqual(child._delegate_role, "leaf")
|
||||
self.assertTrue(any("coercing" in m.lower() for m in cm.output))
|
||||
|
||||
def test_schema_has_role_top_level_and_per_task(self):
|
||||
from tools.delegate_tool import DELEGATE_TASK_SCHEMA
|
||||
props = DELEGATE_TASK_SCHEMA["parameters"]["properties"]
|
||||
self.assertIn("role", props)
|
||||
self.assertEqual(props["role"]["enum"], ["leaf", "orchestrator"])
|
||||
task_props = props["tasks"]["items"]["properties"]
|
||||
self.assertIn("role", task_props)
|
||||
self.assertEqual(task_props["role"]["enum"], ["leaf", "orchestrator"])
|
||||
|
||||
|
||||
# Sentinel used to distinguish "role kwarg omitted" from "role=None".
|
||||
_SENTINEL = object()
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# role-honoring behavior
|
||||
# =========================================================================
|
||||
|
||||
|
||||
def _make_role_mock_child():
|
||||
"""Helper: mock child with minimal fields for delegate_task to process."""
|
||||
mock_child = MagicMock()
|
||||
mock_child.run_conversation.return_value = {
|
||||
"final_response": "done", "completed": True,
|
||||
"api_calls": 1, "messages": [],
|
||||
}
|
||||
mock_child._delegate_saved_tool_names = []
|
||||
mock_child._credential_pool = None
|
||||
mock_child.session_prompt_tokens = 0
|
||||
mock_child.session_completion_tokens = 0
|
||||
mock_child.model = "test"
|
||||
return mock_child
|
||||
|
||||
|
||||
class TestOrchestratorRoleBehavior(unittest.TestCase):
|
||||
"""Tests that role='orchestrator' actually changes toolset + prompt."""
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 2})
|
||||
def test_orchestrator_role_keeps_delegation_at_depth_1(
|
||||
self, mock_cfg, mock_creds
|
||||
):
|
||||
"""role='orchestrator' + depth-0 parent with max_spawn_depth=2 →
|
||||
child at depth 1 gets 'delegation' in enabled_toolsets (can
|
||||
further delegate). Requires max_spawn_depth>=2 since the new
|
||||
default is 1 (flat)."""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent.enabled_toolsets = ["terminal", "file"]
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = _make_role_mock_child()
|
||||
MockAgent.return_value = mock_child
|
||||
delegate_task(goal="test", role="orchestrator", parent_agent=parent)
|
||||
kwargs = MockAgent.call_args[1]
|
||||
self.assertIn("delegation", kwargs["enabled_toolsets"])
|
||||
self.assertEqual(mock_child._delegate_role, "orchestrator")
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 2})
|
||||
def test_orchestrator_blocked_at_max_spawn_depth(
|
||||
self, mock_cfg, mock_creds
|
||||
):
|
||||
"""Parent at depth 1 with max_spawn_depth=2 spawns child
|
||||
at depth 2 (the floor); role='orchestrator' degrades to leaf."""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=1)
|
||||
parent.enabled_toolsets = ["terminal", "delegation"]
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = _make_role_mock_child()
|
||||
MockAgent.return_value = mock_child
|
||||
delegate_task(goal="test", role="orchestrator", parent_agent=parent)
|
||||
kwargs = MockAgent.call_args[1]
|
||||
self.assertNotIn("delegation", kwargs["enabled_toolsets"])
|
||||
self.assertEqual(mock_child._delegate_role, "leaf")
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config", return_value={})
|
||||
def test_orchestrator_blocked_at_default_flat_depth(
|
||||
self, mock_cfg, mock_creds
|
||||
):
|
||||
"""With default max_spawn_depth=1 (flat), role='orchestrator'
|
||||
on a depth-0 parent produces a depth-1 child that is already at
|
||||
the floor — the role degrades to 'leaf' and the delegation
|
||||
toolset is stripped. This is the new default posture."""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent.enabled_toolsets = ["terminal", "file", "delegation"]
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = _make_role_mock_child()
|
||||
MockAgent.return_value = mock_child
|
||||
delegate_task(goal="test", role="orchestrator", parent_agent=parent)
|
||||
kwargs = MockAgent.call_args[1]
|
||||
self.assertNotIn("delegation", kwargs["enabled_toolsets"])
|
||||
self.assertEqual(mock_child._delegate_role, "leaf")
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
def test_orchestrator_enabled_false_forces_leaf(self, mock_creds):
|
||||
"""Kill switch delegation.orchestrator_enabled=false overrides
|
||||
role='orchestrator'."""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent.enabled_toolsets = ["terminal", "delegation"]
|
||||
with patch("tools.delegate_tool._load_config",
|
||||
return_value={"orchestrator_enabled": False}):
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = _make_role_mock_child()
|
||||
MockAgent.return_value = mock_child
|
||||
delegate_task(goal="test", role="orchestrator",
|
||||
parent_agent=parent)
|
||||
kwargs = MockAgent.call_args[1]
|
||||
self.assertNotIn("delegation", kwargs["enabled_toolsets"])
|
||||
self.assertEqual(mock_child._delegate_role, "leaf")
|
||||
|
||||
# ── Role-aware system prompt ────────────────────────────────────────
|
||||
|
||||
def test_leaf_prompt_does_not_mention_delegation(self):
|
||||
prompt = _build_child_system_prompt(
|
||||
"Fix tests", role="leaf",
|
||||
max_spawn_depth=2, child_depth=1,
|
||||
)
|
||||
self.assertNotIn("delegate_task", prompt)
|
||||
self.assertNotIn("Orchestrator Role", prompt)
|
||||
|
||||
def test_orchestrator_prompt_mentions_delegation_capability(self):
|
||||
prompt = _build_child_system_prompt(
|
||||
"Survey approaches", role="orchestrator",
|
||||
max_spawn_depth=2, child_depth=1,
|
||||
)
|
||||
self.assertIn("delegate_task", prompt)
|
||||
self.assertIn("Orchestrator Role", prompt)
|
||||
# Depth/max-depth note present and literal:
|
||||
self.assertIn("depth 1", prompt)
|
||||
self.assertIn("max_spawn_depth=2", prompt)
|
||||
|
||||
def test_orchestrator_prompt_at_depth_floor_says_children_are_leaves(self):
|
||||
"""With max_spawn_depth=2 and child_depth=1, the orchestrator's
|
||||
own children would be at depth 2 (the floor) → must be leaves."""
|
||||
prompt = _build_child_system_prompt(
|
||||
"Survey", role="orchestrator",
|
||||
max_spawn_depth=2, child_depth=1,
|
||||
)
|
||||
self.assertIn("MUST be leaves", prompt)
|
||||
|
||||
def test_orchestrator_prompt_below_floor_allows_more_nesting(self):
|
||||
"""With max_spawn_depth=3 and child_depth=1, the orchestrator's
|
||||
own children can themselves be orchestrators (depth 2 < 3)."""
|
||||
prompt = _build_child_system_prompt(
|
||||
"Deep work", role="orchestrator",
|
||||
max_spawn_depth=3, child_depth=1,
|
||||
)
|
||||
self.assertIn("can themselves be orchestrators", prompt)
|
||||
|
||||
# ── Batch mode and intersection ─────────────────────────────────────
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 2})
|
||||
def test_batch_mode_per_task_role_override(self, mock_cfg, mock_creds):
|
||||
"""Per-task role beats top-level; no top-level role → "leaf".
|
||||
|
||||
tasks=[{role:'orchestrator'},{role:'leaf'},{}] → first gets
|
||||
delegation, second and third don't. Requires max_spawn_depth>=2
|
||||
(raised explicitly here) since the new default is 1 (flat).
|
||||
"""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent.enabled_toolsets = ["terminal", "file", "delegation"]
|
||||
built_toolsets = []
|
||||
|
||||
def _factory(*a, **kw):
|
||||
m = _make_role_mock_child()
|
||||
built_toolsets.append(kw.get("enabled_toolsets"))
|
||||
return m
|
||||
|
||||
with patch("run_agent.AIAgent", side_effect=_factory):
|
||||
delegate_task(
|
||||
tasks=[
|
||||
{"goal": "A", "role": "orchestrator"},
|
||||
{"goal": "B", "role": "leaf"},
|
||||
{"goal": "C"}, # no role → falls back to top_role (leaf)
|
||||
],
|
||||
parent_agent=parent,
|
||||
)
|
||||
self.assertIn("delegation", built_toolsets[0])
|
||||
self.assertNotIn("delegation", built_toolsets[1])
|
||||
self.assertNotIn("delegation", built_toolsets[2])
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 2})
|
||||
def test_intersection_preserves_delegation_bound(
|
||||
self, mock_cfg, mock_creds
|
||||
):
|
||||
"""Design decision: orchestrator capability is granted by role,
|
||||
NOT inherited from the parent's toolset. A parent without
|
||||
'delegation' in its enabled_toolsets can still spawn an
|
||||
orchestrator child — the re-add in _build_child_agent runs
|
||||
unconditionally for orchestrators (when max_spawn_depth allows).
|
||||
|
||||
If you want to change to "parent must have delegation too",
|
||||
update _build_child_agent to check parent_toolsets before the
|
||||
re-add and update this test to match.
|
||||
"""
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent.enabled_toolsets = ["terminal", "file"] # no delegation
|
||||
with patch("run_agent.AIAgent") as MockAgent:
|
||||
mock_child = _make_role_mock_child()
|
||||
MockAgent.return_value = mock_child
|
||||
delegate_task(goal="test", role="orchestrator",
|
||||
parent_agent=parent)
|
||||
self.assertIn("delegation", MockAgent.call_args[1]["enabled_toolsets"])
|
||||
|
||||
|
||||
class TestOrchestratorEndToEnd(unittest.TestCase):
|
||||
"""End-to-end: parent -> orchestrator -> two-leaf nested orchestration.
|
||||
|
||||
Covers the acceptance gate: parent delegates to an orchestrator
|
||||
child; the orchestrator delegates to two leaf grandchildren; the
|
||||
role/toolset/depth chain all resolve correctly.
|
||||
|
||||
Mock strategy: a single AIAgent patch with a side_effect factory
|
||||
that keys on the child's ephemeral_system_prompt — orchestrator
|
||||
prompts contain the string "Orchestrator Role" (see
|
||||
_build_child_system_prompt), leaves don't. The orchestrator
|
||||
mock's run_conversation recursively calls delegate_task with
|
||||
tasks=[{goal:...},{goal:...}] to spawn two leaves. This keeps
|
||||
the test in one patch context and avoids depth-indexed nesting.
|
||||
"""
|
||||
|
||||
@patch("tools.delegate_tool._resolve_delegation_credentials")
|
||||
@patch("tools.delegate_tool._load_config",
|
||||
return_value={"max_spawn_depth": 2})
|
||||
def test_end_to_end_nested_orchestration(self, mock_cfg, mock_creds):
|
||||
mock_creds.return_value = {
|
||||
"provider": None, "base_url": None,
|
||||
"api_key": None, "api_mode": None, "model": None,
|
||||
}
|
||||
parent = _make_mock_parent(depth=0)
|
||||
parent.enabled_toolsets = ["terminal", "file", "delegation"]
|
||||
|
||||
# (enabled_toolsets, _delegate_role) for each agent built
|
||||
built_agents: list = []
|
||||
# Keep the orchestrator mock around so the re-entrant delegate_task
|
||||
# can reach it via closure.
|
||||
orch_mock = {}
|
||||
|
||||
def _factory(*a, **kw):
|
||||
prompt = kw.get("ephemeral_system_prompt", "") or ""
|
||||
is_orchestrator = "Orchestrator Role" in prompt
|
||||
m = _make_role_mock_child()
|
||||
built_agents.append({
|
||||
"enabled_toolsets": list(kw.get("enabled_toolsets") or []),
|
||||
"is_orchestrator_prompt": is_orchestrator,
|
||||
})
|
||||
|
||||
if is_orchestrator:
|
||||
# Prepare the orchestrator mock as a parent-capable object
|
||||
# so the nested delegate_task call succeeds.
|
||||
m._delegate_depth = 1
|
||||
m._delegate_role = "orchestrator"
|
||||
m._active_children = []
|
||||
m._active_children_lock = threading.Lock()
|
||||
m._session_db = None
|
||||
m.platform = "cli"
|
||||
m.enabled_toolsets = ["terminal", "file", "delegation"]
|
||||
m.api_key = "***"
|
||||
m.base_url = ""
|
||||
m.provider = None
|
||||
m.api_mode = None
|
||||
m.providers_allowed = None
|
||||
m.providers_ignored = None
|
||||
m.providers_order = None
|
||||
m.provider_sort = None
|
||||
m._print_fn = None
|
||||
m.tool_progress_callback = None
|
||||
m.thinking_callback = None
|
||||
orch_mock["agent"] = m
|
||||
|
||||
def _orchestrator_run(user_message=None):
|
||||
# Re-entrant: orchestrator spawns two leaves
|
||||
delegate_task(
|
||||
tasks=[{"goal": "leaf-A"}, {"goal": "leaf-B"}],
|
||||
parent_agent=m,
|
||||
)
|
||||
return {
|
||||
"final_response": "orchestrated 2 workers",
|
||||
"completed": True, "api_calls": 1,
|
||||
"messages": [],
|
||||
}
|
||||
m.run_conversation.side_effect = _orchestrator_run
|
||||
|
||||
return m
|
||||
|
||||
with patch("run_agent.AIAgent", side_effect=_factory) as MockAgent:
|
||||
delegate_task(
|
||||
goal="top-level orchestration",
|
||||
role="orchestrator",
|
||||
parent_agent=parent,
|
||||
)
|
||||
|
||||
# 1 orchestrator + 2 leaf grandchildren = 3 agents
|
||||
self.assertEqual(MockAgent.call_count, 3)
|
||||
# First built = the orchestrator (parent's direct child)
|
||||
self.assertIn("delegation", built_agents[0]["enabled_toolsets"])
|
||||
self.assertTrue(built_agents[0]["is_orchestrator_prompt"])
|
||||
# Next two = leaves (grandchildren)
|
||||
self.assertNotIn("delegation", built_agents[1]["enabled_toolsets"])
|
||||
self.assertFalse(built_agents[1]["is_orchestrator_prompt"])
|
||||
self.assertNotIn("delegation", built_agents[2]["enabled_toolsets"])
|
||||
self.assertFalse(built_agents[2]["is_orchestrator_prompt"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue