Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions model_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,54 @@ def _compute_tool_definitions(
]
available_tool_names.discard("browser_exec")

# delegate_task's child-restrictions rule names sibling tools (clarify,
# memory, cronjob). Warning about tools this session doesn't even have
# teaches ghost vocabulary — filter the list to tools actually present
# and drop the line entirely when none apply. Two source variants exist
# (depth-derived): the depth-off line also names delegate_task itself;
# the depth-on line lists only the siblings. Pattern order matters —
# the sibling list is a substring of the full list.
# Same session-level seam as the browser_exec gate above.
if "delegate_task" in available_tool_names:
blocked_present = [
t for t in ("clarify", "memory", "cronjob") if t in available_tool_names
]
if len(blocked_present) < 3:
full_offvariant = "delegate_task, clarify, memory, or cronjob"
full_onvariant = "clarify, memory, or cronjob"
for i, td in enumerate(filtered_tools):
fn = td.get("function", {})
desc = fn.get("description", "")
if fn.get("name") != "delegate_task":
continue
if full_offvariant in desc:
full, keep_self = full_offvariant, True
elif full_onvariant in desc:
full, keep_self = full_onvariant, False
else:
break
names = (["delegate_task"] if keep_self else []) + blocked_present
if blocked_present:
if len(names) == 1:
replacement = names[0]
elif len(names) == 2:
replacement = f"{names[0]} or {names[1]}"
else:
replacement = ", ".join(names[:-1]) + ", or " + names[-1]
desc = desc.replace(full, replacement)
else:
# No sibling tools here — drop the restriction line
# (both variants end at the following "\n").
start = desc.find("- Children cannot call " + full)
if start != -1:
end = desc.index("\n", start) + 1
desc = desc[:start] + desc[end:]
filtered_tools[i] = {
**td,
"function": {**fn, "description": desc},
}
break

if not quiet_mode:
if filtered_tools:
tool_names = [t["function"]["name"] for t in filtered_tools]
Expand Down
53 changes: 37 additions & 16 deletions tests/tools/test_delegate.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,17 @@ class TestDelegateRequirements(unittest.TestCase):
def test_schema_valid(self):
self.assertEqual(DELEGATE_TASK_SCHEMA["name"], "delegate_task")
props = DELEGATE_TASK_SCHEMA["parameters"]["properties"]
self.assertIn("goal", props)
# tasks[] is the only advertised spawn shape (single task = one-entry
# array); legacy top-level goal/context/output_schema stay
# handler-accepted but unadvertised.
self.assertIn("tasks", props)
self.assertIn("context", props)
self.assertNotIn("goal", props)
self.assertNotIn("context", props)
self.assertNotIn("output_schema", props)
task_props = props["tasks"]["items"]["properties"]
self.assertIn("goal", task_props)
self.assertIn("context", task_props)
self.assertIn("output_schema", task_props)
# toolsets is intentionally NOT exposed to the model — subagents always
# inherit the parent's toolsets. Letting the model name toolsets was a
# capability-selection surface the model should not control.
Expand Down Expand Up @@ -101,17 +109,18 @@ def test_top_level_description_compact_and_complete(self):
"context", # pass-everything-via-context rule
"respond in Chinese", # language example (weak models regress without it)
"SELF-REPORTS", # verification contract
"fetch the URL", # concrete verification verbs
"clarify", # leaf blocked-tool list
"send_message",
"clarify", # child blocked-tool list
"delegation.provider", # model inheritance / pinning
):
self.assertIn(keyword, desc, f"top-level description lost: {keyword!r}")
# send_message must NOT be named: gateway-internal vocabulary most
# sessions never see (still enforced via DELEGATE_BLOCKED_TOOLS).
self.assertNotIn("send_message", desc)

def test_dynamic_limits_moved_to_param_descriptions(self):
"""Concurrency and nesting ceilings must reach the model through the
tasks/role parameter descriptions (the top-level text no longer
carries them)."""
"""Concurrency reaches the model through the tasks parameter
description; the depth ceiling lives in the top-level description's
depth-derived recursion rule (role param is gone)."""
from tools.delegate_tool import _build_dynamic_schema_overrides
from tools.registry import registry

Expand All @@ -125,12 +134,11 @@ def test_dynamic_limits_moved_to_param_descriptions(self):

for parameters in (overrides["parameters"], definition["parameters"]):
self.assertIn("up to 7", parameters["properties"]["tasks"]["description"])
self.assertIn(
"max_spawn_depth=4", parameters["properties"]["role"]["description"]
)
# Static top-level text must not embed stale limits.
self.assertNotIn("role", parameters["properties"])
# Depth ceiling now rides the depth-derived recursion rule in the
# top-level text (only rendered when nesting is available).
self.assertIn("max_spawn_depth=4", overrides["description"])
self.assertNotIn("up to 7", overrides["description"])
self.assertNotIn("max_spawn_depth", overrides["description"])

class TestChildSystemPrompt(unittest.TestCase):
def test_goal_only(self):
Expand Down Expand Up @@ -1553,10 +1561,23 @@ def _run_with_mock_child(self, role_arg, mock_cfg, mock_creds):
delegate_task(**kwargs)
return mock_child

def test_default_role_is_leaf(self):
def test_role_is_depth_derived_not_caller_declared(self):
"""With max_spawn_depth=2 (mocked), a depth-1 child has depth budget
left, so it becomes an orchestrator automatically — no role arg
needed, and a passed legacy role arg is ignored either way."""
child = self._run_with_mock_child(_SENTINEL)
self.assertEqual(child._delegate_role, "leaf")

self.assertEqual(child._delegate_role, "orchestrator")
# Legacy explicit role='leaf' does not override the depth derivation.
child = self._run_with_mock_child("leaf")
self.assertEqual(child._delegate_role, "orchestrator")

def test_schema_no_longer_advertises_role(self):
"""`role` left the advertised schema (capability is depth-derived);
the handler still accepts it for wire compat."""
from tools.delegate_tool import DELEGATE_TASK_SCHEMA
props = DELEGATE_TASK_SCHEMA["parameters"]["properties"]
self.assertNotIn("role", props)
self.assertNotIn("role", props["tasks"]["items"]["properties"])

def test_schema_omits_acp_transport_fields(self):
from tools.delegate_tool import DELEGATE_TASK_SCHEMA
Expand Down
16 changes: 11 additions & 5 deletions tests/tools/test_delegate_batch_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,17 @@ def test_placeholder_error_is_actionable(self):


class TestSingleTaskBatch(unittest.TestCase):
def test_one_task_batch_rejected_pointing_to_goal_form(self):
result = _call([{"goal": GOOD_A}])
self.assertIn("error", result)
self.assertIn("goal", result["error"])
self.assertIn("2", result["error"]) # "at least 2"
def test_one_task_batch_is_valid_single_task_shape(self):
"""A one-entry tasks[] array is the canonical single-task call (the
advertised interface is tasks-only), so it must NOT be rejected —
and short goals are legitimate for a single task."""
with patch("tools.delegate_tool._run_single_child") as mock_run:
mock_run.return_value = {
"task_index": 0, "status": "completed", "summary": "done",
"api_calls": 1, "duration_seconds": 1.0, "_child_role": None,
}
result = _call([{"goal": GOOD_A}])
self.assertNotIn("error", result)


class TestValidBatchStillRuns(unittest.TestCase):
Expand Down
12 changes: 6 additions & 6 deletions tests/tools/test_delegate_control_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ def test_delegate_task_unknown_action_is_an_error():

def test_delegate_task_spawn_action_still_validates_goal():
out = delegate_task(action="spawn", parent_agent=_StubParent())
assert "Provide either 'goal'" in out
assert "No tasks provided" in out
assert "one-entry" in out # teaching error carries the canonical shape


def test_delegate_task_requires_parent_agent_for_control():
Expand All @@ -283,12 +284,11 @@ def test_delegate_task_requires_parent_agent_for_control():


def test_empty_tasks_array_with_goal_is_single_task_not_batch_error():
"""Small models emit tasks=[] alongside goal; that must not trip the
'Batch mode requires at least 2 tasks' gate (observed live with
gpt-5.4-mini on Nous Portal)."""
"""Small models emit tasks=[] alongside goal; that must not trip a
batch-count gate (observed live with gpt-5.4-mini on Nous Portal) —
it falls through to the no-tasks teaching error."""
out = delegate_task(tasks=[], goal="", parent_agent=_StubParent())
# Falls through to the single-goal validation, not the batch gate.
assert "Provide either 'goal'" in out
assert "No tasks provided" in out
assert "at least 2 tasks" not in out


Expand Down
10 changes: 7 additions & 3 deletions tests/tools/test_delegate_output_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,14 @@ def test_output_schema_on_task_items(self):
"properties"
]["tasks"]["items"]["required"]

def test_output_schema_on_top_level_goal_form(self):
def test_output_schema_advertised_per_task_only(self):
"""output_schema is advertised inside tasks[] items (the only spawn
shape); the legacy top-level param stays handler-accepted but out
of the schema."""
props = DELEGATE_TASK_SCHEMA["parameters"]["properties"]
assert "output_schema" in props
assert props["output_schema"]["type"] == "object"
assert "output_schema" not in props
task_props = props["tasks"]["items"]["properties"]
assert task_props["output_schema"]["type"] == "object"


# ---------------------------------------------------------------------------
Expand Down
Loading
Loading