mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-01 01:51:44 +00:00
fix(delegate): move _saved_tool_names assignment to correct scope
The merge at e7844e9c re-introduced a line in _build_child_agent() that
references _saved_tool_names — a variable only defined in _run_single_child().
This caused NameError on every delegate_task call, completely breaking
subagent delegation.
Moves the child._delegate_saved_tool_names assignment to _run_single_child()
where _saved_tool_names is actually defined, keeping the save/restore in the
same scope as the try/finally block.
Adds two regression tests from PR #2038 (YanSte).
Also fixes the same issue reported in PR #2048 (Gutslabs).
Co-authored-by: Yannick Stephan <yannick.stephan@gmail.com>
Co-authored-by: Guts <gutslabs@users.noreply.github.com>
This commit is contained in:
parent
d76fa7fc37
commit
ae8059ca24
2 changed files with 54 additions and 2 deletions
|
|
@ -23,6 +23,7 @@ from tools.delegate_tool import (
|
||||||
MAX_DEPTH,
|
MAX_DEPTH,
|
||||||
check_delegate_requirements,
|
check_delegate_requirements,
|
||||||
delegate_task,
|
delegate_task,
|
||||||
|
_build_child_agent,
|
||||||
_build_child_system_prompt,
|
_build_child_system_prompt,
|
||||||
_strip_blocked_tools,
|
_strip_blocked_tools,
|
||||||
_resolve_delegation_credentials,
|
_resolve_delegation_credentials,
|
||||||
|
|
@ -291,6 +292,58 @@ class TestToolNamePreservation(unittest.TestCase):
|
||||||
|
|
||||||
self.assertEqual(model_tools._last_resolved_tool_names, original_tools)
|
self.assertEqual(model_tools._last_resolved_tool_names, original_tools)
|
||||||
|
|
||||||
|
def test_build_child_agent_does_not_raise_name_error(self):
|
||||||
|
"""Regression: _build_child_agent must not reference _saved_tool_names.
|
||||||
|
|
||||||
|
The bug introduced by the e7844e9c merge conflict: line 235 inside
|
||||||
|
_build_child_agent read `list(_saved_tool_names)` where that variable
|
||||||
|
is only defined later in _run_single_child. Calling _build_child_agent
|
||||||
|
standalone (without _run_single_child's scope) must never raise NameError.
|
||||||
|
"""
|
||||||
|
parent = _make_mock_parent(depth=0)
|
||||||
|
|
||||||
|
with patch("run_agent.AIAgent"):
|
||||||
|
try:
|
||||||
|
_build_child_agent(
|
||||||
|
task_index=0,
|
||||||
|
goal="regression check",
|
||||||
|
context=None,
|
||||||
|
toolsets=None,
|
||||||
|
model=None,
|
||||||
|
max_iterations=10,
|
||||||
|
parent_agent=parent,
|
||||||
|
)
|
||||||
|
except NameError as exc:
|
||||||
|
self.fail(
|
||||||
|
f"_build_child_agent raised NameError — "
|
||||||
|
f"_saved_tool_names leaked back into wrong scope: {exc}"
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_saved_tool_names_set_on_child_before_run(self):
|
||||||
|
"""_run_single_child must set _delegate_saved_tool_names on the child
|
||||||
|
from model_tools._last_resolved_tool_names before run_conversation."""
|
||||||
|
import model_tools
|
||||||
|
|
||||||
|
parent = _make_mock_parent(depth=0)
|
||||||
|
expected_tools = ["read_file", "web_search", "execute_code"]
|
||||||
|
model_tools._last_resolved_tool_names = list(expected_tools)
|
||||||
|
|
||||||
|
captured = {}
|
||||||
|
|
||||||
|
with patch("run_agent.AIAgent") as MockAgent:
|
||||||
|
mock_child = MagicMock()
|
||||||
|
|
||||||
|
def capture_and_return(user_message):
|
||||||
|
captured["saved"] = list(mock_child._delegate_saved_tool_names)
|
||||||
|
return {"final_response": "ok", "completed": True, "api_calls": 1}
|
||||||
|
|
||||||
|
mock_child.run_conversation.side_effect = capture_and_return
|
||||||
|
MockAgent.return_value = mock_child
|
||||||
|
|
||||||
|
delegate_task(goal="capture test", parent_agent=parent)
|
||||||
|
|
||||||
|
self.assertEqual(captured["saved"], expected_tools)
|
||||||
|
|
||||||
|
|
||||||
class TestDelegateObservability(unittest.TestCase):
|
class TestDelegateObservability(unittest.TestCase):
|
||||||
"""Tests for enriched metadata returned by _run_single_child."""
|
"""Tests for enriched metadata returned by _run_single_child."""
|
||||||
|
|
|
||||||
|
|
@ -232,8 +232,6 @@ def _build_child_agent(
|
||||||
tool_progress_callback=child_progress_cb,
|
tool_progress_callback=child_progress_cb,
|
||||||
iteration_budget=shared_budget,
|
iteration_budget=shared_budget,
|
||||||
)
|
)
|
||||||
child._delegate_saved_tool_names = list(_saved_tool_names)
|
|
||||||
|
|
||||||
# Set delegation depth so children can't spawn grandchildren
|
# Set delegation depth so children can't spawn grandchildren
|
||||||
child._delegate_depth = getattr(parent_agent, '_delegate_depth', 0) + 1
|
child._delegate_depth = getattr(parent_agent, '_delegate_depth', 0) + 1
|
||||||
|
|
||||||
|
|
@ -270,6 +268,7 @@ def _run_single_child(
|
||||||
# save/restore happens in the same scope as the try/finally.
|
# save/restore happens in the same scope as the try/finally.
|
||||||
import model_tools
|
import model_tools
|
||||||
_saved_tool_names = list(model_tools._last_resolved_tool_names)
|
_saved_tool_names = list(model_tools._last_resolved_tool_names)
|
||||||
|
child._delegate_saved_tool_names = _saved_tool_names
|
||||||
|
|
||||||
try:
|
try:
|
||||||
result = child.run_conversation(user_message=goal)
|
result = child.run_conversation(user_message=goal)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue