fix: resolve subagent tools at execution decision (#381)

* fix: defer subagent tool resolution

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>

* fix: preserve inherited subagent tools

* test: cover injected subagent tools

* style: format subagent regression

---------

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
This commit is contained in:
ADITYA
2026-08-11 18:43:04 +05:30
committed by GitHub
parent fb329e4aaa
commit 1f3e8f57a3
5 changed files with 183 additions and 116 deletions
+31 -19
View File
@@ -442,7 +442,11 @@ def _fold_expert_subagents(subs: list[dict], tool_registry: dict) -> None:
def _maybe_swap_async_subagents(
subs: list, middleware: list | None = None, *, cfg=None
subs: list,
middleware: list | None = None,
*,
tool_registry: dict | None = None,
cfg=None,
) -> list:
"""Replace ``_async``-flagged sub-agents with ``AsyncSubAgent`` specs when enabled.
@@ -458,17 +462,24 @@ def _maybe_swap_async_subagents(
Adding a new async sub-agent requires no change here — flip
``async: true`` in its yaml and create the matching deployment graph.
All return paths strip the internal ``_async`` field from sub-agent dicts
before handoff, since deepagents may schema-validate the kwarg.
YAML tool names stay in the internal ``_tool_names`` field until this
decision point. In-process specs resolve them against ``tool_registry``;
swapped remote specs discard them because their graph factory resolves
tools in its own process. All return paths strip internal fields before
handoff, since deepagents may schema-validate the kwargs.
When async subagents are actually swapped in and ``middleware`` is provided,
appends ``AsyncWatcherMiddleware`` so launches spawn an
``async_notifier`` watcher.
"""
from .utils import resolve_subagent_tools
cfg = cfg if cfg is not None else _ensure_config()
tool_registry = tool_registry or {}
if not getattr(cfg, "enable_async_subagents", False):
# Async fully disabled — strip the internal flag before handoff.
# Async fully disabled: every spec will run in-process.
for s in subs:
resolve_subagent_tools(s, tool_registry)
s.pop("_async", None)
return subs
@@ -482,9 +493,9 @@ def _maybe_swap_async_subagents(
"enable_async_subagents=true but langgraph dev is not reachable; "
"falling back to in-process sync delegation for all sub-agents."
)
# Strip the internal ``_async`` flag (carried from ``load_subagents``)
# before sub-agents reach deepagents — it's never a deepagents key.
# Every spec falls back to in-process execution.
for s in subs:
resolve_subagent_tools(s, tool_registry)
s.pop("_async", None)
return subs
@@ -496,6 +507,7 @@ def _maybe_swap_async_subagents(
if not async_specs:
for s in subs:
resolve_subagent_tools(s, tool_registry)
s.pop("_async", None)
return subs
@@ -526,7 +538,7 @@ def _maybe_swap_async_subagents(
agent_specs[name] = spec
out.append(spec)
else:
# Strip the internal flag before handoff to deepagents.
resolve_subagent_tools(s, tool_registry)
s.pop("_async", None)
out.append(s)
@@ -662,21 +674,20 @@ def _build_base_kwargs(
tool_registry["tavily_search"] = tavily_search
base_tools = [think_tool, skill_manager]
# ``async_swap_pending=True`` because ``_maybe_swap_async_subagents``
# below re-resolves tools for async subagents against the deployed
# graph's own registry (via ``subagents/_factory.py``). A tool missing
# from ``tool_registry`` for an async spec logs at DEBUG, not WARNING.
subs = load_subagents(
SUBAGENTS_CONFIG,
tool_registry=tool_registry,
async_swap_pending=True,
)
_fold_expert_subagents(subs, tool_registry)
_ensure_general_purpose_subagent(subs)
_inject_subagent_middleware(
subs, workspace_dir=workspace_dir, cfg=cfg, chat_model=chat_model
)
subs = _maybe_swap_async_subagents(subs, base_middleware, cfg=cfg)
subs = _maybe_swap_async_subagents(
subs,
base_middleware,
tool_registry=tool_registry,
cfg=cfg,
)
# Route AsyncSubAgent specs (both standard and expert) through
# EvoAsyncSubAgentMiddleware so the payload-aware start_async_task tool
# replaces upstream's non-parameterisable one.
@@ -746,12 +757,8 @@ def load_mcp_and_build_kwargs(
mcp_main = mcp_by_agent.pop("main", [])
# Same rationale as ``_build_base_kwargs``: async subagents get
# re-resolved downstream by the deployed graph's factory registry.
subs = load_subagents(
SUBAGENTS_CONFIG,
tool_registry=registry,
async_swap_pending=True,
)
_fold_expert_subagents(subs, registry)
@@ -767,7 +774,12 @@ def load_mcp_and_build_kwargs(
# Swap selected sub-agents to AsyncSubAgent (must happen AFTER MCP injection
# since async sub-agents are remote graphs that load their own tools).
subs = _maybe_swap_async_subagents(subs, base_middleware, cfg=cfg)
subs = _maybe_swap_async_subagents(
subs,
base_middleware,
tool_registry=registry,
cfg=cfg,
)
# Mirror the base path: route AsyncSubAgent specs through
# EvoAsyncSubAgentMiddleware so the payload-aware start_async_task tool
# is the one composed into the main agent.
+2 -2
View File
@@ -52,7 +52,7 @@ def build_async_subagent_graph(name: str) -> Any:
_inject_subagent_middleware,
)
from EvoScientist.tools import skill_manager, tavily_search, think_tool
from EvoScientist.utils import load_subagents
from EvoScientist.utils import load_subagents, resolve_subagent_tools
# Surface API keys as env vars so downstream SDKs (openai, anthropic, …)
# find them on subprocess invocations from langgraph dev.
@@ -68,7 +68,6 @@ def build_async_subagent_graph(name: str) -> Any:
# all wired the same way as the in-process sync version.
specs = load_subagents(
SUBAGENTS_CONFIG,
tool_registry=tool_registry,
)
spec = next((s for s in specs if s.get("name") == name), None)
if spec is None:
@@ -76,6 +75,7 @@ def build_async_subagent_graph(name: str) -> Any:
f"Sub-agent {name!r} not found in {SUBAGENTS_CONFIG}. "
f"Available: {[s.get('name') for s in specs]}"
)
resolve_subagent_tools(spec, tool_registry)
# Load MCP tools routed to THIS agent via ``expose_to: <name>`` in
# ``mcp.yaml``. Use the cached helper so multiple ``build_async_subagent_graph``
+36 -48
View File
@@ -18,6 +18,28 @@ logger = logging.getLogger(__name__)
console = Console()
def resolve_subagent_tools(
subagent: dict[str, Any], tool_registry: dict[str, Any]
) -> dict[str, Any]:
"""Resolve deferred YAML tool names while preserving injected tool objects."""
tool_names = subagent.pop("_tool_names", None)
if tool_names is None:
return subagent
resolved = list(subagent.get("tools", []))
for tool_name in tool_names:
if tool_name in tool_registry:
resolved.append(tool_registry[tool_name])
else:
logger.warning(
"Subagent %r: tool %r not in registry, skipping",
subagent.get("name"),
tool_name,
)
subagent["tools"] = resolved
return subagent
def format_message_content(message):
"""Convert message content to displayable string.
@@ -112,11 +134,9 @@ def show_prompt(prompt_text: str, title: str = "Prompt", border_style: str = "bl
def load_subagents(
config_path: Path,
*,
tool_registry: dict[str, Any],
prompt_refs: dict[str, str] | None = None,
async_swap_pending: bool = False,
) -> list[dict[str, Any]]:
"""Load subagent definitions from a directory of YAML files and wire up tools.
"""Load subagent definitions from a directory of YAML files.
NOTE: This is a custom utility. deepagents does not natively load subagents
from files - they're normally defined inline in the create_deep_agent() call.
@@ -141,16 +161,9 @@ def load_subagents(
system_prompt: |
...
``async_swap_pending`` tells the loader that an async-subagent swap will
run downstream against a different tool registry (typically the deployed
graph's own registry in ``subagents/_factory.py``). When True, a tool
missing from ``tool_registry`` for an ``async: true`` spec is logged at
DEBUG (it will be re-resolved by the async graph). When False, the
caller IS the terminal registry — any missing tool is a genuine typo
and logs at WARNING. Sync in-process callers
(``EvoScientist._build_base_kwargs``, ``load_mcp_and_build_kwargs``)
should pass True; the factory
(``subagents/_factory.build_async_subagent_graph``) leaves it False.
Tool names are retained until the caller selects an execution mode. This
avoids resolving async specs against the in-process registry; the selected
in-process or remote graph resolves names against its terminal registry.
"""
prompt_refs = prompt_refs or {}
@@ -224,9 +237,14 @@ def load_subagents(
if "skills" in spec:
subagent["skills"] = spec["skills"]
# Compute the async flag up-front so the tools-resolution block below
# can pick the right log level for missing tools. Internal field:
# carries the ``async:`` yaml flag through to
# Defer YAML-name resolution until the caller decides whether this
# spec will run in-process. Async specs are replaced by remote graph
# references and must not warn against the caller's unrelated registry.
if "tools" in spec:
subagent["_tool_names"] = list(spec["tools"])
subagent["tools"] = []
# Internal field: carries the ``async:`` yaml flag through to
# ``_maybe_swap_async_subagents`` so the swap doesn't need a second
# yaml pass to discover async-flagged agents. Underscore prefix marks
# it as internal — must be popped before passing to deepagents.
@@ -240,30 +258,6 @@ def load_subagents(
f"got {type(async_val).__name__}: {async_val!r}"
)
if "tools" in spec:
resolved = []
for t in spec["tools"]:
if t in tool_registry:
resolved.append(tool_registry[t])
elif async_val and async_swap_pending:
# Caller expects an async-subagent swap downstream against
# a different registry. The in-process registry is
# intentionally minimal for async agents — a miss here is
# not degraded state.
logger.debug(
"Subagent %r: tool %r not in the in-process registry; "
"resolved by the async graph",
name,
t,
)
else:
# Terminal registry (factory) OR sync subagent — a missing
# tool is a genuine typo and won't be resolved anywhere.
logger.warning(
"Subagent %r: tool %r not in registry, skipping", name, t
)
subagent["tools"] = resolved
subagent["_async"] = async_val
return subagent
@@ -280,18 +274,12 @@ def load_subagent(
*,
tool_registry: dict[str, Any],
prompt_refs: dict[str, str] | None = None,
async_swap_pending: bool = False,
) -> dict[str, Any]:
"""Load a single sub-agent by name from YAML.
See :func:`load_subagents` for ``async_swap_pending`` semantics.
"""
"""Load and resolve a single sub-agent by name from YAML."""
for agent in load_subagents(
config_path,
tool_registry=tool_registry,
prompt_refs=prompt_refs,
async_swap_pending=async_swap_pending,
):
if agent.get("name") == name:
return agent
return resolve_subagent_tools(agent, tool_registry)
raise KeyError(f"Sub-agent not found: {name}")
+33 -1
View File
@@ -13,13 +13,20 @@ from unittest.mock import patch
from EvoScientist.EvoScientist import _maybe_swap_async_subagents
def _sub(name: str, *, async_flag: bool, description: str = "desc") -> dict:
def _sub(
name: str,
*,
async_flag: bool,
description: str = "desc",
tool_names: list[str] | None = None,
) -> dict:
"""Build a sub-agent dict shaped like ``utils.load_subagents`` output."""
return {
"name": name,
"description": description,
"system_prompt": "x",
"tools": [],
"_tool_names": tool_names or [],
"_async": async_flag,
}
@@ -45,6 +52,7 @@ def test_returns_unchanged_when_async_disabled_and_strips_flag():
assert out is subs
for s in out:
assert "_async" not in s, f"_async leaked into {s['name']}"
assert "_tool_names" not in s
# =============================================================================
@@ -158,6 +166,30 @@ def test_swaps_async_flagged_subs():
assert data["url"] == "http://127.0.0.1:6174"
def test_only_in_process_specs_resolve_against_the_caller_registry():
cfg = SimpleNamespace(enable_async_subagents=True, langgraph_dev_port=6174)
sync_tool = object()
subs = [
_sub("planner-agent", async_flag=False, tool_names=["think_tool"]),
_sub("writing-agent", async_flag=True, tool_names=["remote_only"]),
]
with patch(
"EvoScientist.langgraph_dev.manager.is_async_subagents_available",
return_value=True,
):
out = _maybe_swap_async_subagents(
subs,
tool_registry={"think_tool": sync_tool},
cfg=cfg,
)
planner = next(s for s in out if s["name"] == "planner-agent")
assert planner["tools"] == [sync_tool]
assert "_tool_names" not in planner
writing = next(s for s in out if s["name"] == "writing-agent")
assert writing["graph_id"] == "writing-agent"
def test_swap_uses_configured_port():
"""AsyncSubAgent.url should reflect cfg.langgraph_dev_port, not hardcoded."""
cfg = SimpleNamespace(enable_async_subagents=True, langgraph_dev_port=9999)
+81 -46
View File
@@ -11,7 +11,7 @@ import textwrap
import pytest
from EvoScientist.utils import load_subagents
from EvoScientist.utils import load_subagents, resolve_subagent_tools
def _write_yaml(tmp_path, name: str, body: str):
@@ -33,7 +33,7 @@ def test_async_flag_accepts_real_bool(tmp_path):
async: true
""",
)
subs = load_subagents(config_path, tool_registry={})
subs = load_subagents(config_path)
assert len(subs) == 1
assert subs[0]["name"] == "writing-agent"
assert subs[0]["_async"] is True
@@ -51,10 +51,67 @@ def test_async_flag_defaults_to_false_when_omitted(tmp_path):
tools: []
""",
)
subs = load_subagents(config_path, tool_registry={})
subs = load_subagents(config_path)
assert subs[0]["_async"] is False
def test_tool_names_are_deferred_until_the_spec_is_selected(tmp_path, caplog):
selected_tool = object()
config_path = _write_yaml(
tmp_path,
"agents.yaml",
"""
selected-agent:
tools: [available]
remote-agent:
tools: [remote_only]
async: true
""",
)
subs = load_subagents(config_path)
assert not caplog.records
assert subs[0]["tools"] == []
assert subs[0]["_tool_names"] == ["available"]
resolve_subagent_tools(subs[0], {"available": selected_tool})
assert subs[0]["tools"] == [selected_tool]
assert "_tool_names" not in subs[0]
def test_resolve_subagent_tools_preserves_injected_tools():
injected = object()
selected_tool = object()
subagent = {
"name": "selected-agent",
"tools": [injected],
"_tool_names": ["available"],
}
resolve_subagent_tools(subagent, {"available": selected_tool})
assert subagent["tools"] == [injected, selected_tool]
assert "_tool_names" not in subagent
def test_agent_without_tools_preserves_parent_tool_inheritance(tmp_path):
config_path = _write_yaml(
tmp_path,
"general.yaml",
"""
general-purpose:
description: Handles general tasks
system_prompt: ""
""",
)
subagent = load_subagents(config_path)[0]
resolve_subagent_tools(subagent, {"available": object()})
assert "tools" not in subagent
assert "_tool_names" not in subagent
def test_async_flag_rejects_quoted_string(tmp_path):
"""``async: "false"`` (quoted) is a real user trap — bool("false") is True.
@@ -73,7 +130,7 @@ def test_async_flag_rejects_quoted_string(tmp_path):
""",
)
with pytest.raises(ValueError, match=r"'async' must be a boolean"):
load_subagents(config_path, tool_registry={})
load_subagents(config_path)
def test_async_flag_rejects_integer(tmp_path):
@@ -90,7 +147,7 @@ def test_async_flag_rejects_integer(tmp_path):
""",
)
with pytest.raises(ValueError, match=r"'async' must be a boolean"):
load_subagents(config_path, tool_registry={})
load_subagents(config_path)
def test_async_flag_error_includes_agent_name(tmp_path):
@@ -107,7 +164,7 @@ def test_async_flag_error_includes_agent_name(tmp_path):
""",
)
with pytest.raises(ValueError, match=r"my-bad-agent"):
load_subagents(config_path, tool_registry={})
load_subagents(config_path)
def test_non_dict_spec_raises(tmp_path):
@@ -125,7 +182,7 @@ def test_non_dict_spec_raises(tmp_path):
""",
)
with pytest.raises(ValueError, match=r"must map to a spec dict"):
load_subagents(config_path, tool_registry={})
load_subagents(config_path)
def test_non_dict_spec_error_includes_filename_and_name(tmp_path):
@@ -138,16 +195,11 @@ def test_non_dict_spec_error_includes_filename_and_name(tmp_path):
""",
)
with pytest.raises(ValueError, match=r"weird\.yaml.*weird-agent"):
load_subagents(config_path, tool_registry={})
load_subagents(config_path)
def test_missing_tool_on_sync_subagent_logs_warning(tmp_path, caplog):
"""Sync sub-agents with a tool missing from the registry log at WARNING.
Sync sub-agents run in-process under the main agent and rely on the
in-process registry to wire every tool they declare. A missing tool
IS a genuine degradation — surfaces it as a warning.
"""
def test_missing_tool_on_sync_subagent_warns_at_resolution(tmp_path, caplog):
"""A selected sync spec warns when its terminal registry lacks a tool."""
config_path = _write_yaml(
tmp_path,
"planner.yaml",
@@ -159,23 +211,19 @@ def test_missing_tool_on_sync_subagent_logs_warning(tmp_path, caplog):
""",
)
with caplog.at_level("DEBUG", logger="EvoScientist.utils"):
subs = load_subagents(config_path, tool_registry={})
subs = load_subagents(config_path)
assert subs[0]["_async"] is False
assert subs[0]["tools"] == []
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
assert not warnings
resolve_subagent_tools(subs[0], {})
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
assert any("nonexistent_tool" in r.getMessage() for r in warnings)
def test_missing_tool_on_async_subagent_logs_debug_when_swap_pending(tmp_path, caplog):
"""Sync in-process callers pass ``async_swap_pending=True`` — a tool
missing for an ``async: true`` spec logs at DEBUG because the async
graph's own registry will re-resolve it downstream
(``subagents/_factory.py``).
Regression guard for the spurious startup WARNING that pre-fix logs
fired on every ``EvoSci`` startup even though the tool was wired at
runtime by the deployed graph.
"""
def test_missing_tool_on_async_subagent_is_deferred_without_logging(tmp_path, caplog):
"""Async tool names do not emit warnings against the caller registry."""
config_path = _write_yaml(
tmp_path,
"scheduler.yaml",
@@ -188,31 +236,18 @@ def test_missing_tool_on_async_subagent_logs_debug_when_swap_pending(tmp_path, c
""",
)
with caplog.at_level("DEBUG", logger="EvoScientist.utils"):
subs = load_subagents(config_path, tool_registry={}, async_swap_pending=True)
subs = load_subagents(config_path)
assert subs[0]["_async"] is True
assert subs[0]["tools"] == []
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
assert not any("nonexistent_tool" in r.getMessage() for r in warnings)
debugs = [r for r in caplog.records if r.levelname == "DEBUG"]
assert any(
"nonexistent_tool" in r.getMessage() and "async graph" in r.getMessage()
for r in debugs
)
assert not any("nonexistent_tool" in r.getMessage() for r in debugs)
assert subs[0]["_tool_names"] == ["nonexistent_tool"]
def test_missing_tool_on_async_subagent_logs_warning_at_terminal_registry(
tmp_path, caplog
):
"""When ``async_swap_pending`` is False (the default), the caller IS the
terminal registry — the factory boundary
(``subagents/_factory.build_async_subagent_graph``). A tool missing for
an ``async: true`` spec is a genuine typo that won't be resolved anywhere
downstream, so log at WARNING.
Without this guard, an earlier version of the fix (unconditional DEBUG for
every async spec) silently hid factory-boundary typos. Reviewer flagged
this as the last remaining gap.
"""
def test_missing_async_tool_warns_at_terminal_resolution(tmp_path, caplog):
"""The selected async graph warns against its terminal registry."""
config_path = _write_yaml(
tmp_path,
"scheduler.yaml",
@@ -225,8 +260,8 @@ def test_missing_tool_on_async_subagent_logs_warning_at_terminal_registry(
""",
)
with caplog.at_level("DEBUG", logger="EvoScientist.utils"):
# Default: async_swap_pending=False → factory-boundary semantics.
subs = load_subagents(config_path, tool_registry={})
subs = load_subagents(config_path)
resolve_subagent_tools(subs[0], {})
assert subs[0]["_async"] is True
assert subs[0]["tools"] == []
warnings = [r for r in caplog.records if r.levelname == "WARNING"]