From d7f655f370e2bea177dfedf19f3b261d13c986e2 Mon Sep 17 00:00:00 2001 From: tymrtn Date: Wed, 3 Jun 2026 23:22:21 +0200 Subject: [PATCH] fix: accept typed clarify choice replies --- gateway/platforms/base.py | 15 ++-- gateway/run.py | 22 ++--- .../test_clarify_active_session_bypass.py | 87 +++++++++++++++++++ tests/tools/test_clarify_gateway.py | 26 ++++++ tools/clarify_gateway.py | 46 ++++++++-- 5 files changed, 174 insertions(+), 22 deletions(-) create mode 100644 tests/gateway/test_clarify_active_session_bypass.py diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 616d5a73c85..4066868fa69 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -4410,11 +4410,11 @@ class BasePlatformAdapter(ABC): logger.error("[%s] Command '/%s' dispatch failed: %s", self.name, cmd, e, exc_info=True) return - # Clarify text-capture bypass: if the agent is blocked on a - # clarify_tool call awaiting a free-form text response (open- - # ended clarify, or user picked "Other"), the next non-command - # message in this session MUST reach the runner so the - # clarify-intercept can resolve it and unblock the agent. + # Clarify reply bypass: if the agent is blocked on a + # clarify_tool call, the next non-command message in this + # session MUST reach the runner so typed numeric choices, + # exact choices, and free-form "Other" answers can resolve the + # clarify-intercept and unblock the agent. # # Without this bypass: the message gets queued in # _pending_messages as a follow-up turn instead of reaching the @@ -4427,7 +4427,10 @@ class BasePlatformAdapter(ABC): try: from tools import clarify_gateway as _clarify_mod _has_text_clarify = ( - _clarify_mod.get_pending_for_session(session_key) is not None + _clarify_mod.get_pending_for_session( + session_key, + include_choice_prompts=True, + ) is not None ) except Exception: _has_text_clarify = False diff --git a/gateway/run.py b/gateway/run.py index 9e55fe40466..429f1ce0b54 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -8037,26 +8037,28 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew ) _update_prompts.pop(_quick_key, None) - # Intercept messages that are responses to a pending clarify - # request that is awaiting free-form text (either an open-ended - # clarify with no choices, or one where the user picked the - # "Other" button). The first non-empty user message in the - # session resolves the clarify and unblocks the agent thread — - # we do NOT route it to the agent as a new turn. + # Intercept messages that are responses to a pending clarify. + # Open-ended prompts and "Other" responses are captured as free text; + # direct replies to multi-choice prompts are accepted too ("2" maps + # to the second option, arbitrary text becomes a custom answer). Slash + # commands still bypass this path so /stop and friends keep working. + _clarify_mod = None try: from tools import clarify_gateway as _clarify_mod - _pending_clarify = _clarify_mod.get_pending_for_session(_quick_key) + _pending_clarify = _clarify_mod.get_pending_for_session( + _quick_key, include_choice_prompts=True, + ) except Exception: _pending_clarify = None - if _pending_clarify is not None: + if _pending_clarify is not None and _clarify_mod is not None: _raw_clarify_reply = (event.text or "").strip() # Skip slash commands — the user clearly wanted to issue a # command, not answer the clarify. Leave the clarify pending # so the user can retry; if it times out, the agent unblocks # with an empty response. if _raw_clarify_reply and not _raw_clarify_reply.startswith("/"): - _resolved = _clarify_mod.resolve_gateway_clarify( - _pending_clarify.clarify_id, _raw_clarify_reply, + _resolved = _clarify_mod.resolve_text_response_for_session( + _quick_key, _raw_clarify_reply, ) if _resolved: logger.info( diff --git a/tests/gateway/test_clarify_active_session_bypass.py b/tests/gateway/test_clarify_active_session_bypass.py new file mode 100644 index 00000000000..bfa5ffff561 --- /dev/null +++ b/tests/gateway/test_clarify_active_session_bypass.py @@ -0,0 +1,87 @@ +"""Regression tests for clarify replies while a gateway session is busy.""" + +import asyncio +from unittest.mock import AsyncMock + +import pytest + +from gateway.config import Platform, PlatformConfig +from gateway.platforms.base import ( + BasePlatformAdapter, + MessageEvent, + MessageType, + SendResult, +) +from gateway.session import SessionSource, build_session_key + + +class _ClarifyBypassAdapter(BasePlatformAdapter): + def __init__(self): + super().__init__(PlatformConfig(enabled=True, token="test"), Platform.TELEGRAM) + + async def connect(self): + return True + + async def disconnect(self): + pass + + async def send(self, chat_id, content, reply_to=None, metadata=None): + return SendResult(success=True, message_id="text") + + async def get_chat_info(self, chat_id): + return {"id": chat_id, "type": "private"} + + +def _event(text="custom answer"): + return MessageEvent( + text=text, + message_type=MessageType.TEXT, + source=SessionSource( + platform=Platform.TELEGRAM, + chat_id="12345", + chat_type="private", + user_id="user1", + ), + message_id="msg1", + ) + + +def _clear_clarify_state(): + from tools import clarify_gateway as cm + + with cm._lock: + cm._entries.clear() + cm._session_index.clear() + cm._notify_cbs.clear() + + +@pytest.mark.asyncio +async def test_active_session_routes_typed_choice_clarify_reply_to_runner_not_busy_queue(): + """Typed text must resolve a pending choice clarify even while the agent is busy. + + Telegram button clarifies keep the adapter session active while the agent + thread blocks on ``wait_for_response``. If the adapter only bypasses for + entries already marked ``awaiting_text``, typed replies to the visible + multi-choice prompt are handled as busy follow-ups and the clarify wait is + never resolved. + """ + _clear_clarify_state() + from tools import clarify_gateway as cm + + adapter = _ClarifyBypassAdapter() + adapter._message_handler = AsyncMock(return_value="") + adapter._busy_session_handler = AsyncMock(return_value=True) + event = _event("None of those are valid options") + session_key = build_session_key( + event.source, + group_sessions_per_user=adapter.config.extra.get("group_sessions_per_user", True), + thread_sessions_per_user=adapter.config.extra.get("thread_sessions_per_user", False), + ) + adapter._active_sessions[session_key] = asyncio.Event() + cm.register("clarify-1", session_key, "Pick one", ["A", "B"]) + + await adapter.handle_message(event) + + adapter._message_handler.assert_awaited_once_with(event) + adapter._busy_session_handler.assert_not_awaited() + assert adapter._pending_messages == {} diff --git a/tests/tools/test_clarify_gateway.py b/tests/tools/test_clarify_gateway.py index 5f999676f42..bd44ecdb961 100644 --- a/tests/tools/test_clarify_gateway.py +++ b/tests/tools/test_clarify_gateway.py @@ -64,6 +64,32 @@ class TestClarifyPrimitive: assert entry.awaiting_text is False assert cm.get_pending_for_session("sk3") is None + def test_include_choice_prompts_returns_multi_choice_entry(self): + """Gateway typed replies must see active choice prompts too.""" + from tools import clarify_gateway as cm + + cm.register("id3b", "sk3b", "Pick", ["X", "Y"]) + pending = cm.get_pending_for_session("sk3b", include_choice_prompts=True) + assert pending is not None + assert pending.clarify_id == "id3b" + + def test_resolve_text_response_maps_numeric_choice(self): + """Typed numbers should resolve to the canonical choice string.""" + from tools import clarify_gateway as cm + + cm.register("id3c", "sk3c", "Pick", ["X", "Y"]) + assert cm.resolve_text_response_for_session("sk3c", "2") is True + assert cm.wait_for_response("id3c", timeout=0.1) == "Y" + + def test_resolve_text_response_accepts_custom_other_text(self): + """Arbitrary typed text should resolve as a custom Other answer.""" + from tools import clarify_gateway as cm + + cm.register("id3d", "sk3d", "Pick", ["X", "Y"]) + custom = "None of those are valid options" + assert cm.resolve_text_response_for_session("sk3d", custom) is True + assert cm.wait_for_response("id3d", timeout=0.1) == custom + def test_other_button_flips_to_text_mode(self): """mark_awaiting_text makes get_pending_for_session find the entry.""" from tools import clarify_gateway as cm diff --git a/tools/clarify_gateway.py b/tools/clarify_gateway.py index e3bed96779b..676530261b1 100644 --- a/tools/clarify_gateway.py +++ b/tools/clarify_gateway.py @@ -162,12 +162,19 @@ def resolve_gateway_clarify(clarify_id: str, response: str) -> bool: return True -def get_pending_for_session(session_key: str) -> Optional[_ClarifyEntry]: - """Return the OLDEST pending clarify entry for a session, or None. +def get_pending_for_session( + session_key: str, + *, + include_choice_prompts: bool = False, +) -> Optional[_ClarifyEntry]: + """Return the oldest pending clarify entry for a session, or None. - Used by the text-fallback intercept in ``_handle_message`` — when a - clarify is awaiting a free-form text response, the next user message - in that session is captured as the answer. + By default this only returns entries awaiting free-form text (open-ended + clarifies, or a multi-choice clarify after the user picked ``Other``). + Gateways may pass ``include_choice_prompts=True`` when the user has typed + directly in response to an active multi-choice prompt; in that case the + oldest unresolved clarify is returned so the text can resolve it instead + of being queued as an unrelated follow-up turn. """ with _lock: ids = _session_index.get(session_key) or [] @@ -175,11 +182,38 @@ def get_pending_for_session(session_key: str) -> Optional[_ClarifyEntry]: entry = _entries.get(cid) if entry is None: continue - if entry.awaiting_text: + if include_choice_prompts or entry.awaiting_text: return entry return None +def _coerce_text_response(entry: _ClarifyEntry, response: str) -> str: + """Map typed choice replies to canonical choice text, otherwise keep custom text.""" + text = str(response).strip() + if entry.choices: + try: + idx = int(text) - 1 + except ValueError: + idx = -1 + if 0 <= idx < len(entry.choices): + return entry.choices[idx] + for choice in entry.choices: + if text.casefold() == str(choice).strip().casefold(): + return str(choice).strip() + return text + + +def resolve_text_response_for_session(session_key: str, response: str) -> bool: + """Resolve the oldest pending clarify in ``session_key`` from typed text.""" + entry = get_pending_for_session(session_key, include_choice_prompts=True) + if entry is None: + return False + return resolve_gateway_clarify( + entry.clarify_id, + _coerce_text_response(entry, response), + ) + + def mark_awaiting_text(clarify_id: str) -> bool: """Flip an entry into text-capture mode (user picked the 'Other' button).