diff --git a/scripts/release.py b/scripts/release.py index 68d9e44528b38..a3f01adf0c7f4 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -1617,6 +1617,7 @@ AUTHOR_MAP = { "claw@openclaw.ai": "wanwan2qq", # PR #10215 (strip brackets/quotes from /resume; gateway session-ID lookup) "simo.kiihamaki@gmail.com": "SimoKiihamaki", # PR #30773 (Windows /reset+/new freeze; stdin fallback for modal) "66773372+Tranquil-Flow@users.noreply.github.com": "Tranquil-Flow", # PR #27518 (bracketed-paste timeout) + "uriyas22@gmail.com": "riyas22", # PR #43687 salvage (strip cronjob toolset from delegated children, #43466) "8bit64k@pm.me": "8bit64k", # PR #14681 (TUI /q alias from quit to queue) "chenglunhu@gmail.com": "hclsys", # PR #31985 (TUI /q alias regression test) "dearmayo@localhost": "ffr31mr", # PR #32103 (SubdirectoryHintTracker workspace boundary) diff --git a/tests/tools/test_delegate.py b/tests/tools/test_delegate.py index 911025e9993b4..b6ba4c66f7a05 100644 --- a/tests/tools/test_delegate.py +++ b/tests/tools/test_delegate.py @@ -156,6 +156,37 @@ class TestStripBlockedTools(unittest.TestCase): result = _strip_blocked_tools([]) self.assertEqual(result, []) + def test_strips_cronjob_toolset(self): + """Regression for issue #43466: child subagents must not inherit + the cronjob toolset from a parent running on a gateway platform. + Without this guard, a delegated child could schedule new cron jobs + under the parent's identity. + """ + result = _strip_blocked_tools( + ["terminal", "file", "cronjob", "web"] + ) + self.assertNotIn("cronjob", result) + self.assertIn("terminal", result) + self.assertIn("file", result) + self.assertIn("web", result) + + def test_strip_set_derived_from_blocklist(self): + """The strip set must be derived from DELEGATE_BLOCKED_TOOLS so a + new blocked tool can't silently leak through as a toolset name + (regression for issue #43466's 'more robust variant' suggestion). + """ + from tools.delegate_tool import TOOLSETS, _strip_blocked_tools + # Every toolset whose tools are ALL in the blocklist should be stripped + for name, defn in TOOLSETS.items(): + tools = defn.get("tools", []) + if tools and all(t in DELEGATE_BLOCKED_TOOLS for t in tools): + self.assertNotIn( + name, + _strip_blocked_tools([name, "terminal"]), + f"Toolset {name!r} (tools={tools}) is fully blocked " + f"but was not stripped", + ) + class TestDelegateTask(unittest.TestCase): def test_no_parent_agent(self): diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 1be02f240e07a..04cf67f4f1a64 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -49,6 +49,7 @@ DELEGATE_BLOCKED_TOOLS = frozenset( "memory", # no writes to shared MEMORY.md "send_message", # no cross-platform side effects "execute_code", # children should reason step-by-step, not write scripts + "cronjob", # no scheduling more work in the parent's name ] ) @@ -766,12 +767,21 @@ def _resolve_workspace_hint(parent_agent) -> Optional[str]: def _strip_blocked_tools(toolsets: List[str]) -> List[str]: - """Remove toolsets that contain only blocked tools.""" + """Remove toolsets that contain only blocked tools. + + The strip set is derived from DELEGATE_BLOCKED_TOOLS plus the explicit + composite/scenario toolsets (delegation, code_execution) that have no + one-to-one tool. This keeps the blocklist and the strip set in lockstep + so new blocked tools can't silently leak through as toolset names. + """ + # Composite toolsets that should never pass through to children, even + # though their individual tools aren't all in DELEGATE_BLOCKED_TOOLS. + _COMPOSITE_BLOCKED_TOOLSETS = frozenset({"delegation", "code_execution"}) blocked_toolset_names = { - "delegation", - "clarify", - "memory", - "code_execution", + name + for name, defn in TOOLSETS.items() + if name in _COMPOSITE_BLOCKED_TOOLSETS + or all(t in DELEGATE_BLOCKED_TOOLS for t in defn.get("tools", [])) } return [t for t in toolsets if t not in blocked_toolset_names]