mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(gateway): resume agent after /approve executes blocked command
When a dangerous command was blocked and the user approved it via /approve, the command was executed but the agent loop had already exited — the agent never received the command output and the task died silently. Now _handle_approve_command sends immediate feedback to the user, then creates a synthetic continuation message with the command output and feeds it through _handle_message so the agent picks up where it left off. - Send command result to chat immediately via adapter.send() - Create synthetic MessageEvent with command + output as context - Spawn asyncio task to re-invoke agent via _handle_message - Return None (feedback already sent directly) - Add test for agent re-invocation after approval - Update existing approval tests for new return behavior
This commit is contained in:
Nick
Teknium
parent
8327f7cc61
commit
9a581bba50
2 changed files with 128 additions and 8 deletions
|
|
@ -24,6 +24,7 @@ import signal
|
|||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
from logging.handlers import RotatingFileHandler
|
||||
from pathlib import Path
|
||||
from datetime import datetime
|
||||
|
|
@ -4720,9 +4721,13 @@ class GatewayRunner:
|
|||
|
||||
_APPROVAL_TIMEOUT_SECONDS = 300 # 5 minutes
|
||||
|
||||
async def _handle_approve_command(self, event: MessageEvent) -> str:
|
||||
async def _handle_approve_command(self, event: MessageEvent) -> Optional[str]:
|
||||
"""Handle /approve command — execute a pending dangerous command.
|
||||
|
||||
After execution, re-invokes the agent with the command result so it
|
||||
can continue its multi-step task (fixes the "dead agent" bug where
|
||||
the agent loop exited on approval_required and never resumed).
|
||||
|
||||
Usage:
|
||||
/approve — approve and execute the pending command
|
||||
/approve session — approve and remember for this session
|
||||
|
|
@ -4771,8 +4776,57 @@ class GatewayRunner:
|
|||
|
||||
logger.info("User approved dangerous command via /approve: %s...%s", cmd[:60], scope_msg)
|
||||
from tools.terminal_tool import terminal_tool
|
||||
result = terminal_tool(command=cmd, force=True)
|
||||
return f"✅ Command approved and executed{scope_msg}.\n\n```\n{result[:3500]}\n```"
|
||||
result = await asyncio.to_thread(terminal_tool, command=cmd, force=True)
|
||||
|
||||
# Send immediate feedback so the user sees the command output right away
|
||||
immediate_msg = f"✅ Command approved and executed{scope_msg}.\n\n```\n{result[:3500]}\n```"
|
||||
adapter = self.adapters.get(source.platform)
|
||||
if adapter:
|
||||
try:
|
||||
await adapter.send(source.chat_id, immediate_msg)
|
||||
except Exception as e:
|
||||
logger.warning("Failed to send approval feedback: %s", e)
|
||||
|
||||
# Re-invoke the agent with the command result so it can continue its task.
|
||||
# The agent's conversation history (persisted in SQLite) already contains
|
||||
# the tool call that returned approval_required — the continuation message
|
||||
# provides the actual execution output so the agent can pick up where it
|
||||
# left off.
|
||||
continuation_text = (
|
||||
f"[System: The user approved the previously blocked command and it has been executed.\n"
|
||||
f"Command: {cmd}\n"
|
||||
f"<command_output>\n{result[:3500]}\n</command_output>\n\n"
|
||||
f"Continue with the task you were working on.]"
|
||||
)
|
||||
|
||||
synthetic_event = MessageEvent(
|
||||
text=continuation_text,
|
||||
source=source,
|
||||
message_id=f"approve-continuation-{uuid.uuid4().hex}",
|
||||
)
|
||||
|
||||
async def _continue_agent():
|
||||
try:
|
||||
response = await self._handle_message(synthetic_event)
|
||||
if response and adapter:
|
||||
await adapter.send(source.chat_id, response)
|
||||
except Exception as e:
|
||||
logger.error("Failed to continue agent after /approve: %s", e)
|
||||
if adapter:
|
||||
try:
|
||||
await adapter.send(
|
||||
source.chat_id,
|
||||
f"⚠️ Failed to resume agent after approval: {e}"
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
_task = asyncio.create_task(_continue_agent())
|
||||
self._background_tasks.add(_task)
|
||||
_task.add_done_callback(self._background_tasks.discard)
|
||||
# Return None — we already sent the immediate feedback and the agent
|
||||
# continuation is running in the background.
|
||||
return None
|
||||
|
||||
async def _handle_deny_command(self, event: MessageEvent) -> str:
|
||||
"""Handle /deny command — reject a pending dangerous command."""
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ Verifies that dangerous command approvals require explicit /approve or /deny
|
|||
slash commands, not bare "yes"/"no" text matching.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
|
@ -49,6 +50,7 @@ def _make_runner():
|
|||
runner._running_agents = {}
|
||||
runner._pending_messages = {}
|
||||
runner._pending_approvals = {}
|
||||
runner._background_tasks = set()
|
||||
runner._session_db = None
|
||||
runner._reasoning_config = None
|
||||
runner._provider_routing = {}
|
||||
|
|
@ -78,20 +80,32 @@ class TestApproveCommand:
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approve_executes_pending_command(self):
|
||||
"""Basic /approve executes the pending command."""
|
||||
"""Basic /approve executes the pending command and sends feedback."""
|
||||
runner = _make_runner()
|
||||
source = _make_source()
|
||||
session_key = runner._session_key_for_source(source)
|
||||
runner._pending_approvals[session_key] = _make_pending_approval()
|
||||
|
||||
event = _make_event("/approve")
|
||||
with patch("tools.terminal_tool.terminal_tool", return_value="done") as mock_term:
|
||||
with (
|
||||
patch("tools.terminal_tool.terminal_tool", return_value="done") as mock_term,
|
||||
patch.object(runner, "_handle_message", new_callable=AsyncMock, return_value="agent continued"),
|
||||
):
|
||||
result = await runner._handle_approve_command(event)
|
||||
# Yield to let the background continuation task run.
|
||||
# This works because mocks return immediately (no real await points).
|
||||
await asyncio.sleep(0)
|
||||
|
||||
assert "✅ Command approved and executed" in result
|
||||
# Returns None because feedback is sent directly via adapter
|
||||
assert result is None
|
||||
mock_term.assert_called_once_with(command="sudo rm -rf /tmp/test", force=True)
|
||||
assert session_key not in runner._pending_approvals
|
||||
|
||||
# Immediate feedback sent via adapter
|
||||
adapter = runner.adapters[Platform.TELEGRAM]
|
||||
sent_text = adapter.send.call_args_list[0][0][1]
|
||||
assert "Command approved and executed" in sent_text
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approve_session_remembers_pattern(self):
|
||||
"""/approve session approves the pattern for the session."""
|
||||
|
|
@ -104,12 +118,21 @@ class TestApproveCommand:
|
|||
with (
|
||||
patch("tools.terminal_tool.terminal_tool", return_value="done"),
|
||||
patch("tools.approval.approve_session") as mock_session,
|
||||
patch.object(runner, "_handle_message", new_callable=AsyncMock, return_value=None),
|
||||
):
|
||||
result = await runner._handle_approve_command(event)
|
||||
# Yield to let the background continuation task run.
|
||||
# This works because mocks return immediately (no real await points).
|
||||
await asyncio.sleep(0)
|
||||
|
||||
assert "pattern approved for this session" in result
|
||||
assert result is None
|
||||
mock_session.assert_called_once_with(session_key, "sudo")
|
||||
|
||||
# Verify scope message in adapter feedback
|
||||
adapter = runner.adapters[Platform.TELEGRAM]
|
||||
sent_text = adapter.send.call_args_list[0][0][1]
|
||||
assert "pattern approved for this session" in sent_text
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approve_always_approves_permanently(self):
|
||||
"""/approve always approves the pattern permanently."""
|
||||
|
|
@ -122,12 +145,21 @@ class TestApproveCommand:
|
|||
with (
|
||||
patch("tools.terminal_tool.terminal_tool", return_value="done"),
|
||||
patch("tools.approval.approve_permanent") as mock_perm,
|
||||
patch.object(runner, "_handle_message", new_callable=AsyncMock, return_value=None),
|
||||
):
|
||||
result = await runner._handle_approve_command(event)
|
||||
# Yield to let the background continuation task run.
|
||||
# This works because mocks return immediately (no real await points).
|
||||
await asyncio.sleep(0)
|
||||
|
||||
assert "pattern approved permanently" in result
|
||||
assert result is None
|
||||
mock_perm.assert_called_once_with("sudo")
|
||||
|
||||
# Verify scope message in adapter feedback
|
||||
adapter = runner.adapters[Platform.TELEGRAM]
|
||||
sent_text = adapter.send.call_args_list[0][0][1]
|
||||
assert "pattern approved permanently" in sent_text
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approve_no_pending(self):
|
||||
"""/approve with no pending approval returns helpful message."""
|
||||
|
|
@ -152,6 +184,40 @@ class TestApproveCommand:
|
|||
assert "expired" in result
|
||||
assert session_key not in runner._pending_approvals
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approve_reinvokes_agent_with_result(self):
|
||||
"""After executing, /approve re-invokes the agent with command output."""
|
||||
runner = _make_runner()
|
||||
source = _make_source()
|
||||
session_key = runner._session_key_for_source(source)
|
||||
runner._pending_approvals[session_key] = _make_pending_approval()
|
||||
|
||||
event = _make_event("/approve")
|
||||
mock_handle = AsyncMock(return_value="I continued the task.")
|
||||
|
||||
with (
|
||||
patch("tools.terminal_tool.terminal_tool", return_value="file deleted"),
|
||||
patch.object(runner, "_handle_message", mock_handle),
|
||||
):
|
||||
await runner._handle_approve_command(event)
|
||||
# Yield to let the background continuation task run.
|
||||
# This works because mocks return immediately (no real await points).
|
||||
await asyncio.sleep(0)
|
||||
|
||||
# Agent was re-invoked via _handle_message with a synthetic event
|
||||
mock_handle.assert_called_once()
|
||||
synthetic_event = mock_handle.call_args[0][0]
|
||||
assert "approved" in synthetic_event.text.lower()
|
||||
assert "file deleted" in synthetic_event.text
|
||||
assert "sudo rm -rf /tmp/test" in synthetic_event.text
|
||||
|
||||
# The continuation response was sent to the user
|
||||
adapter = runner.adapters[Platform.TELEGRAM]
|
||||
# First call: immediate feedback, second call: agent continuation
|
||||
assert adapter.send.call_count == 2
|
||||
continuation_response = adapter.send.call_args_list[1][0][1]
|
||||
assert continuation_response == "I continued the task."
|
||||
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# /deny command
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue