diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 71da29aee5..90d98490c5 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -796,6 +796,10 @@ delegation: # Raise to 2 to allow workers to spawn their own subagents. # Requires role="orchestrator" on intermediate agents. # orchestrator_enabled: true # Kill switch for role="orchestrator" children (default: true). + # subagent_auto_approve: false # When a subagent hits a dangerous-command approval prompt, auto-deny (default: false) + # or auto-approve "once" (true) instead of blocking on stdin. + # The parent TUI owns stdin, so blocking would deadlock; non-interactive resolution is required. + # Both choices emit a logger.warning audit line. Flip to true only for cron/batch pipelines. # inherit_mcp_toolsets: true # When explicit child toolsets are narrowed, also keep the parent's MCP toolsets (default: true). Set false for strict intersection. # model: "google/gemini-3-flash-preview" # Override model for subagents (empty = inherit parent) # provider: "openrouter" # Override provider for subagents (empty = inherit parent) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 7678287a0e..fe59e80f0e 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -783,6 +783,15 @@ DEFAULT_CONFIG = { # warning log if out of range. "max_spawn_depth": 1, # depth cap (1 = flat [default], 2 = orchestrator→leaf, 3 = three-level) "orchestrator_enabled": True, # kill switch for role="orchestrator" + # When a subagent hits a dangerous-command approval prompt, the parent's + # prompt_toolkit TUI owns stdin — a thread-local input() call from the + # subagent worker would deadlock the parent UI. To avoid the deadlock, + # subagent threads ALWAYS resolve approvals non-interactively: + # false (default) → auto-deny with a logger.warning audit line (safe) + # true → auto-approve "once" with a logger.warning audit line + # Flip to true only if you trust delegated work to run dangerous cmds + # without human review (cron pipelines, batch automation, etc.). + "subagent_auto_approve": False, }, # Ephemeral prefill messages file — JSON list of {role, content} dicts diff --git a/tests/tools/test_delegate.py b/tests/tools/test_delegate.py index f3a1a2632d..c27908da8f 100644 --- a/tests/tools/test_delegate.py +++ b/tests/tools/test_delegate.py @@ -2128,5 +2128,103 @@ class TestOrchestratorEndToEnd(unittest.TestCase): self.assertFalse(built_agents[2]["is_orchestrator_prompt"]) +class TestSubagentApprovalCallback(unittest.TestCase): + """Subagent worker threads must have a non-interactive approval callback + installed so dangerous-command prompts don't fall back to input() and + deadlock the parent's prompt_toolkit TUI. + + Governed by delegation.subagent_auto_approve: + false (default) → _subagent_auto_deny + true → _subagent_auto_approve + """ + + def test_auto_deny_returns_deny(self): + from tools.delegate_tool import _subagent_auto_deny + self.assertEqual( + _subagent_auto_deny("rm -rf /tmp/x", "dangerous"), + "deny", + ) + + def test_auto_approve_returns_once(self): + from tools.delegate_tool import _subagent_auto_approve + self.assertEqual( + _subagent_auto_approve("rm -rf /tmp/x", "dangerous"), + "once", + ) + + @patch("tools.delegate_tool._load_config", return_value={}) + def test_getter_defaults_to_deny(self, _mock_cfg): + from tools.delegate_tool import ( + _get_subagent_approval_callback, + _subagent_auto_deny, + ) + self.assertIs(_get_subagent_approval_callback(), _subagent_auto_deny) + + @patch( + "tools.delegate_tool._load_config", + return_value={"subagent_auto_approve": False}, + ) + def test_getter_explicit_false_is_deny(self, _mock_cfg): + from tools.delegate_tool import ( + _get_subagent_approval_callback, + _subagent_auto_deny, + ) + self.assertIs(_get_subagent_approval_callback(), _subagent_auto_deny) + + @patch( + "tools.delegate_tool._load_config", + return_value={"subagent_auto_approve": True}, + ) + def test_getter_true_is_approve(self, _mock_cfg): + from tools.delegate_tool import ( + _get_subagent_approval_callback, + _subagent_auto_approve, + ) + self.assertIs(_get_subagent_approval_callback(), _subagent_auto_approve) + + @patch( + "tools.delegate_tool._load_config", + return_value={"subagent_auto_approve": "yes"}, + ) + def test_getter_truthy_string_is_approve(self, _mock_cfg): + """is_truthy_value accepts 'yes'/'1'/'true' as truthy.""" + from tools.delegate_tool import ( + _get_subagent_approval_callback, + _subagent_auto_approve, + ) + self.assertIs(_get_subagent_approval_callback(), _subagent_auto_approve) + + def test_executor_initializer_installs_callback_in_worker(self): + """The initializer sets the callback on the worker thread's TLS, + not the parent's — verifies the fix actually scopes to workers. + """ + from concurrent.futures import ThreadPoolExecutor + from tools.terminal_tool import ( + set_approval_callback as _set_cb, + _get_approval_callback, + ) + from tools.delegate_tool import _subagent_auto_deny + + # Parent thread has no callback. + _set_cb(None) + self.assertIsNone(_get_approval_callback()) + + seen = [] + + def worker(): + seen.append(_get_approval_callback()) + + with ThreadPoolExecutor( + max_workers=1, + initializer=_set_cb, + initargs=(_subagent_auto_deny,), + ) as executor: + executor.submit(worker).result() + + self.assertEqual(seen, [_subagent_auto_deny]) + # Parent's callback slot is still empty (TLS isolates threads). + self.assertIsNone(_get_approval_callback()) + + if __name__ == "__main__": unittest.main() diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 357452599f..abdec4717f 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -33,6 +33,7 @@ from typing import Any, Dict, List, Optional from toolsets import TOOLSETS from tools import file_state +from tools.terminal_tool import set_approval_callback as _set_subagent_approval_cb from utils import base_url_hostname, is_truthy_value @@ -47,6 +48,64 @@ DELEGATE_BLOCKED_TOOLS = frozenset( ] ) + +# --------------------------------------------------------------------------- +# Subagent approval callbacks +# --------------------------------------------------------------------------- +# Subagents run inside a ThreadPoolExecutor worker. The CLI's interactive +# approval callback is stored in tools/terminal_tool.py's threading.local(), +# so worker threads do NOT inherit it. Without a callback, +# prompt_dangerous_approval() falls back to input() from the worker thread, +# which deadlocks against the parent's prompt_toolkit TUI that owns stdin. +# +# Fix: install a non-interactive callback into every subagent worker thread +# via ThreadPoolExecutor(initializer=_set_subagent_approval_cb, initargs=(cb,)). +# The callback is chosen by the `delegation.subagent_auto_approve` config: +# false (default) → _subagent_auto_deny (safe; matches leaf tool blocklist) +# true → _subagent_auto_approve (opt-in YOLO for cron/batch) +# Both emit a logger.warning for audit; gateway sessions are unaffected +# because they resolve approvals via tools/approval.py's per-session queue, +# not through these TLS callbacks. +def _subagent_auto_deny(command: str, description: str, **kwargs) -> str: + """Auto-deny dangerous commands in subagent threads (safe default). + + Returns 'deny' so the subagent sees a refusal it can recover from, and + never calls input() (which would deadlock the parent TUI). + """ + logger.warning( + "Subagent auto-denied dangerous command: %s (%s). " + "Set delegation.subagent_auto_approve: true to allow.", + command, description, + ) + return "deny" + + +def _subagent_auto_approve(command: str, description: str, **kwargs) -> str: + """Auto-approve dangerous commands in subagent threads (opt-in YOLO). + + Only installed when delegation.subagent_auto_approve=true. Returns 'once' + so the subagent proceeds without blocking the parent UI. + """ + logger.warning( + "Subagent auto-approved dangerous command: %s (%s)", + command, description, + ) + return "once" + + +def _get_subagent_approval_callback(): + """Return the callback to install into subagent worker threads. + + Config key: delegation.subagent_auto_approve (bool, default False). + Reads via the same _load_config() path as the rest of delegate_task so + priority is config.yaml > (no env override for this knob) > default. + """ + cfg = _load_config() + val = cfg.get("subagent_auto_approve", False) + if is_truthy_value(val): + return _subagent_auto_approve + return _subagent_auto_deny + # Build a description fragment listing toolsets available for subagents. # Excludes toolsets where ALL tools are blocked, composite/platform toolsets # (hermes-* prefixed), and scenario toolsets. @@ -1344,7 +1403,15 @@ def _run_single_child( # Run child with a hard timeout to prevent indefinite blocking # when the child's API call or tool-level HTTP request hangs. child_timeout = _get_child_timeout() - _timeout_executor = ThreadPoolExecutor(max_workers=1) + _timeout_executor = ThreadPoolExecutor( + max_workers=1, + # Install a non-interactive approval callback in the worker thread + # so dangerous-command prompts from the subagent don't fall back to + # input() and deadlock the parent's prompt_toolkit TUI. + # Callback (deny vs approve) is governed by delegation.subagent_auto_approve. + initializer=_set_subagent_approval_cb, + initargs=(_get_subagent_approval_callback(),), + ) # Capture the worker thread so the timeout diagnostic can dump its # Python stack (see #14726 — 0-API-call hangs are opaque without it). _worker_thread_holder: Dict[str, Optional[threading.Thread]] = {"t": None}