diff --git a/gateway/run.py b/gateway/run.py index 49135ce5af..81018722cf 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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"\n{result[:3500]}\n\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.""" diff --git a/tests/gateway/test_approve_deny_commands.py b/tests/gateway/test_approve_deny_commands.py index 3b713eaed5..b63b522234 100644 --- a/tests/gateway/test_approve_deny_commands.py +++ b/tests/gateway/test_approve_deny_commands.py @@ -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