fix(gateway): route plain-text approval responses (salvage #46924)

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:

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()