diff --git a/gateway/run.py b/gateway/run.py index b6737ad16d1a..bed0cbff762c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -4732,6 +4732,81 @@ async def _handle_active_session_busy_message(self, event: MessageEvent, session ) return True + # --- Approval response routing (#46866) --- + # When the agent is blocked waiting for a dangerous-command approval, + # plain-text responses like "yes" or "approve" must be routed to the + # approval handler instead of being steered/queued/interrupted. + # Otherwise approval via messaging platforms never succeeds — the + # reply is queued behind a turn that can't start until the approval + # resolves, so the approval times out and auto-denies (a deadlock). + # + # Slash forms (/approve, /deny) already bypass to the runner at the + # base-adapter guard. This handles the bare-word forms (Signal/SMS + # users naturally type "yes" rather than "/approve"). Gating on + # has_blocking_approval(session_key) is the disambiguator that keeps + # a conversational "yes" from triggering a dangerous command when no + # approval is actually pending (design intent — see run.py "Pending + # exec approvals are handled by /approve and /deny" note). + # + # We reuse the canonical /approve and /deny handlers rather than + # re-deriving the resolution + i18n messaging: they resolve the + # waiting thread, resume typing, AND return a localized confirmation + # string. The busy-handler path does not auto-send that return, so + # we deliver it ourselves (mirroring the draining-case send above). + try: + from tools.approval import has_blocking_approval + if has_blocking_approval(session_key): + _raw_text = (event.text or "").strip().lower() + _approve_words = {"approve", "yes", "ok", "okay", "confirm", "y", "👍"} + _deny_words = {"deny", "no", "reject", "cancel", "n", "👎"} + _approval_handler = None + _normalized_args = "" + if _raw_text in _approve_words: + _approval_handler = self._handle_approve_command + elif _raw_text in _deny_words: + _approval_handler = self._handle_deny_command + elif _raw_text in {"always", "approve always", "always approve"}: + _approval_handler = self._handle_approve_command + _normalized_args = "always" + elif _raw_text in {"session", "approve session", "session approve"}: + _approval_handler = self._handle_approve_command + _normalized_args = "session" + if _approval_handler is not None: + # Synthesize the canonical "/approve [args]" / "/deny" + # command text so the slash handlers parse modifiers via + # event.get_command_args(). Always use a literal "/" — + # MessageEvent.is_command()/get_command_args() only + # recognize the "/" prefix, not the per-platform display + # prefix ("!" on Slack/Matrix). + _verb = "approve" if _approval_handler is self._handle_approve_command else "deny" + _synth = f"/{_verb}" + if _normalized_args: + _synth = f"{_synth} {_normalized_args}" + event.text = _synth + _reply = await _approval_handler(event) + logger.info( + "Approval response via plain text: session=%s verb=%s args=%r", + session_key, _verb, _normalized_args, + ) + _adapter = self.adapters.get(event.source.platform) + if _adapter and _reply: + _text, _eph_ttl = _adapter._unwrap_ephemeral(_reply) + if _text: + _anchor = self._reply_anchor_for_event(event) + await _adapter._send_with_retry( + chat_id=event.source.chat_id, + content=_text, + reply_to=_anchor, + metadata=self._thread_metadata_for_source(event.source, _anchor), + ) + return True + except Exception: + logger.warning( + "Plain-text approval routing failed for session %s; " + "falling through to busy handling", + session_key, exc_info=True, + ) + # Normal busy case (agent actively running a task) adapter = self.adapters.get(event.source.platform) if not adapter: diff --git a/tests/gateway/test_plaintext_approval_routing.py b/tests/gateway/test_plaintext_approval_routing.py new file mode 100644 index 000000000000..9f07be64808a --- /dev/null +++ b/tests/gateway/test_plaintext_approval_routing.py @@ -0,0 +1,196 @@ +"""Tests for #46866: plain-text approval responses must resolve a blocking +dangerous-command approval instead of being steered/queued. + +When the agent is blocked inside tools/approval.py waiting for a dangerous +command to be approved, a messaging user who replies "yes" / "approve" / +"deny" (without the leading slash) must have that response routed to the +approval handler. Previously the bare-word reply fell through to the +steer/queue/interrupt logic in _handle_active_session_busy_message — the +approval never resolved, timed out, and auto-denied. + +Slash forms (/approve, /deny) already bypass at the base-adapter guard; +this covers the bare-word forms Signal/SMS users naturally type. +""" + +import asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import GatewayConfig, Platform, PlatformConfig +from gateway.platforms.base import MessageEvent, MessageType +from gateway.session import SessionSource + + +def _make_source() -> SessionSource: + return SessionSource( + platform=Platform.TELEGRAM, + user_id="u1", + chat_id="c1", + user_name="tester", + chat_type="dm", + ) + + +def _make_event(text: str) -> MessageEvent: + return MessageEvent( + text=text, + message_type=MessageType.TEXT, + source=_make_source(), + message_id="m1", + ) + + +def _clear_approval_state(): + from tools import approval as mod + mod._gateway_queues.clear() + mod._gateway_notify_cbs.clear() + mod._session_approved.clear() + mod._permanent_approved.clear() + mod._pending.clear() + + +def _make_runner(): + """Minimal GatewayRunner that exercises the real busy-session handler.""" + from gateway.run import GatewayRunner + + runner = object.__new__(GatewayRunner) + runner.config = GatewayConfig( + platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="***")} + ) + adapter = MagicMock() + adapter.send = AsyncMock() + adapter._send_with_retry = AsyncMock( + return_value=SimpleNamespace(success=True, message_id="reply1") + ) + # _unwrap_ephemeral is a real base-adapter method; emulate its contract. + adapter._unwrap_ephemeral = lambda r: (r, 0) if isinstance(r, str) else (None, 0) + runner.adapters = {Platform.TELEGRAM: adapter} + runner._running_agents = {} + runner._running_agents_ts = {} + runner._pending_messages = {} + runner._pending_approvals = {} + runner._busy_ack_ts = {} + runner._draining = False + runner.session_store = None + runner._is_user_authorized = lambda _source: True + # _handle_active_session_busy_message uses these only on the + # non-approval fall-through path; harmless to stub. + runner._busy_input_mode = "interrupt" + runner._busy_text_mode = "interrupt" + return runner, adapter + + +def _register_blocking_approval(runner): + """Register a real blocking approval entry for the runner's session.""" + from tools.approval import _ApprovalEntry, _gateway_queues + source = _make_source() + session_key = runner._session_key_for_source(source) + entry = _ApprovalEntry({"command": "rm -rf /tmp/test"}) + _gateway_queues.setdefault(session_key, []).append(entry) + return session_key, entry + + +@pytest.mark.parametrize("reply", ["yes", "approve", "ok", "y", "confirm"]) +def test_plaintext_yes_resolves_approval(reply): + _clear_approval_state() + runner, adapter = _make_runner() + session_key, entry = _register_blocking_approval(runner) + + handled = asyncio.run( + runner._handle_active_session_busy_message(_make_event(reply), session_key) + ) + + assert handled is True + assert entry.event.is_set() + assert entry.result == "once" + # The user gets a confirmation reply, not silence. + adapter._send_with_retry.assert_awaited() + _clear_approval_state() + + +@pytest.mark.parametrize("reply", ["no", "deny", "reject", "n", "cancel"]) +def test_plaintext_no_denies_approval(reply): + _clear_approval_state() + runner, adapter = _make_runner() + session_key, entry = _register_blocking_approval(runner) + + handled = asyncio.run( + runner._handle_active_session_busy_message(_make_event(reply), session_key) + ) + + assert handled is True + assert entry.event.is_set() + assert entry.result == "deny" + adapter._send_with_retry.assert_awaited() + _clear_approval_state() + + +def test_plaintext_always_maps_to_permanent_choice(): + _clear_approval_state() + runner, adapter = _make_runner() + session_key, entry = _register_blocking_approval(runner) + + handled = asyncio.run( + runner._handle_active_session_busy_message(_make_event("always"), session_key) + ) + + assert handled is True + assert entry.result == "always" + _clear_approval_state() + + +def test_plaintext_session_maps_to_session_choice(): + _clear_approval_state() + runner, adapter = _make_runner() + session_key, entry = _register_blocking_approval(runner) + + handled = asyncio.run( + runner._handle_active_session_busy_message(_make_event("session"), session_key) + ) + + assert handled is True + assert entry.result == "session" + _clear_approval_state() + + +def test_no_pending_approval_does_not_consume_conversational_yes(): + """A bare 'yes' with NO blocking approval must NOT be treated as an + approval — it falls through to normal busy handling (design intent: + 'yes' in conversation must not execute a dangerous command).""" + _clear_approval_state() + runner, adapter = _make_runner() + source = _make_source() + session_key = runner._session_key_for_source(source) + # No approval registered. + + handled = asyncio.run( + runner._handle_active_session_busy_message(_make_event("yes"), session_key) + ) + + # No approval existed, so nothing was resolved — the "yes" is treated + # as ordinary text, not as a dangerous-command approval (design intent). + # (It still flows through normal busy handling, which may send a busy + # ack; the contract here is only that no approval was consumed.) + from tools.approval import _gateway_queues + assert session_key not in _gateway_queues + _clear_approval_state() + + +def test_unrelated_text_with_pending_approval_falls_through(): + """Text that is neither approve nor deny vocab must NOT resolve the + approval — it falls through to normal busy handling.""" + _clear_approval_state() + runner, adapter = _make_runner() + session_key, entry = _register_blocking_approval(runner) + + handled = asyncio.run( + runner._handle_active_session_busy_message( + _make_event("what files are here?"), session_key + ) + ) + + # Approval still pending — not resolved by unrelated text. + assert not entry.event.is_set() + _clear_approval_state()