diff --git a/gateway/run_turn_runner_clarify_delivery.py b/gateway/run_turn_runner_clarify_delivery.py index 399fc285b8..1370d1b397 100644 --- a/gateway/run_turn_runner_clarify_delivery.py +++ b/gateway/run_turn_runner_clarify_delivery.py @@ -94,7 +94,10 @@ def _clarify_send_then_wait(fut, *, clarify_id: str, session_key: str, clarify_m if outcome == "failed" and fallback is not None: # The text prompt is the last resort: a late failure of ITS send has nothing to retry. fut, fallback = fallback(), None - outcome = _approval_send_outcome(fut, timeout=SEND_ACK_WINDOW) + # ``None`` = the fallback could not even be scheduled; the card failure already stands, + # so re-classifying would only log a misleading "no scheduling future". + if fut is not None: + outcome = _approval_send_outcome(fut, timeout=SEND_ACK_WINDOW) if outcome == "sent": logger.info("Clarify card undeliverable; plain-text prompt sent instead (id=%s)", clarify_id) abort = _abort_for_outcome(outcome, session_key=session_key, clarify_mod=clarify_mod) @@ -106,7 +109,7 @@ def _clarify_send_then_wait(fut, *, clarify_id: str, session_key: str, clarify_m response = clarify_mod.wait_for_response(clarify_id, timeout=float(timeout)) late.disarm() if late.undeliverable: - return UNDELIVERED, False + return late.undeliverable, False if response is None or response == "": return f"[user did not respond within {int(timeout / 60)}m]", False return response, True @@ -117,11 +120,12 @@ class _LateFailureWatch: ack window, try the text fallback once, then release the waiter with the delivery notice. Callbacks run on the gateway loop thread (the send future completes there); ``clear_session`` - wakes the agent thread blocked in ``wait_for_response`` and ``undeliverable`` tells it why. + wakes the agent thread blocked in ``wait_for_response`` and ``undeliverable`` (the delivery + sentinel, or ``None`` while nothing definitive happened) tells it why. Armed only while the future is still pending: a sent card needs no watch.""" def __init__(self, fut, *, clarify_id: str, session_key: str, clarify_mod, fallback) -> None: - self.undeliverable = False + self.undeliverable: Optional[str] = None self._armed = False self._clarify_id = clarify_id self._session_key = session_key @@ -149,7 +153,17 @@ class _LateFailureWatch: if outcome == "sent": return logger.warning("Clarify card send resolved %s after the ack window (id=%s)", outcome, self._clarify_id) - fallback_fut = self._fallback() if outcome == "failed" and self._fallback is not None else None + if outcome == "ambiguous": + # Lost ack (``raw_response.ambiguous``): the card may well have posted. Same invariant as + # ``_abort_for_outcome`` — stay armed for the late button tap, never re-send, never + # release; the bounded wait's own timeout covers a card that truly never arrived. + return + if outcome == "declined": + # Refused destination: no text retry (see ``_clarify_send_then_wait``), and the notice + # says so rather than the generic delivery failure. + self._release(UNDELIVERED_DECLINED) + return + fallback_fut = self._fallback() if self._fallback is not None else None if fallback_fut is None: self._release() return @@ -163,6 +177,6 @@ class _LateFailureWatch: return self._release() - def _release(self) -> None: - self.undeliverable = True + def _release(self, notice: str = UNDELIVERED) -> None: + self.undeliverable = notice self._clarify_mod.clear_session(self._session_key) diff --git a/tests/gateway/test_clarify_delivery_fallback.py b/tests/gateway/test_clarify_delivery_fallback.py index a88a55ee35..3d429bc5df 100644 --- a/tests/gateway/test_clarify_delivery_fallback.py +++ b/tests/gateway/test_clarify_delivery_fallback.py @@ -78,10 +78,14 @@ def _runner(adapter, loop, monkeypatch, timeout=5): def _answer_once_text_prompt_is_seen(adapter, text): + """Answer ONLY after the plain-text prompt was observed — never on a deadline, so a missing + fallback leaves the waiter blocked and the test fails on elapsed time / sent_text, not luck.""" def _wait_then_answer(): deadline = time.monotonic() + 10 while time.monotonic() < deadline and not adapter.sent_text: time.sleep(0.02) + if not adapter.sent_text: + return time.sleep(0.1) cm.resolve_text_response_for_session("sk-fallback", text) threading.Thread(target=_wait_then_answer, daemon=True).start() @@ -130,8 +134,44 @@ def test_card_failing_after_the_ack_window_falls_back_to_text_instead_of_waiting _answer_once_text_prompt_is_seen(adapter, "beta") started = time.monotonic() response = _runner(adapter, loop, monkeypatch, timeout=30)._clarify_callback_sync("Pick?", ["alpha", "beta"]) + elapsed = time.monotonic() - started assert response == "beta" - assert time.monotonic() - started < 10 # never the full clarify_timeout + assert len(adapter.sent_text) == 1 # the plain-text prompt was actually sent, once + assert elapsed < 0.2 + 2 # released right after the ack window + late failure, never clarify_timeout + + +def test_card_resolving_ambiguous_after_the_ack_window_stays_armed_for_a_button_tap(loop, monkeypatch): + """A relay lost-ack (``raw_response.ambiguous``) after the window means the card MAY have posted: + the registration must stay armed so the user's later button tap still answers — the same + invariant ``_abort_for_outcome`` keeps for an immediate ambiguous outcome. Before: the late watch + treated it like a definitive failure, released the wait with the delivery notice and the tap was + lost (no pending entry).""" + from gateway import run_turn_runner_clarify_delivery as delivery + + monkeypatch.setattr(delivery, "SEND_ACK_WINDOW", 0.2) + + async def late_ambiguous(): + await asyncio.sleep(0.6) + return SendResult(success=False, raw_response={"ambiguous": True}) + + adapter = _CardAdapter(late_ambiguous) + pending_at_tap = [] + + def _tap_button(): + time.sleep(1.5) + entry = cm.get_pending_for_session("sk-fallback", include_choice_prompts=True) + pending_at_tap.append(entry) + if entry is not None: + cm.resolve_gateway_clarify(entry.clarify_id, "beta") + tap = threading.Thread(target=_tap_button, daemon=True) + tap.start() + + response, answered = _runner(adapter, loop, monkeypatch, timeout=30)._ask_clarify_question( + "Pick?", ["alpha", "beta"], False) + tap.join(timeout=5) # never let a late tap leak into the next test's registration + assert pending_at_tap and pending_at_tap[0] is not None # still armed when the tap arrived + assert (response, answered) == ("beta", True) + assert adapter.sent_text == [] # possibly-delivered: never re-sent as text def test_card_and_text_both_failing_late_release_the_wait_with_the_delivery_notice(loop, monkeypatch): @@ -160,6 +200,24 @@ def test_card_and_text_both_failing_late_release_the_wait_with_the_delivery_noti assert len(adapter.sent_text) == 1 # the text fallback was tried exactly once +def test_card_declined_after_the_ack_window_releases_with_the_declined_notice(loop, monkeypatch): + """A late connector DECLINE is as definitive as an immediate one: no text retry, and the + notice names the refusal (``UNDELIVERED_DECLINED``) instead of the generic delivery failure.""" + from gateway import run_turn_runner_clarify_delivery as delivery + + monkeypatch.setattr(delivery, "SEND_ACK_WINDOW", 0.2) + + async def late_decline(): + await asyncio.sleep(0.6) + return SendResult(success=False, error="egress declined: destination not allowed") + + adapter = _CardAdapter(late_decline) + response, answered = _runner(adapter, loop, monkeypatch, timeout=30)._ask_clarify_question( + "Pick?", ["alpha", "beta"], False) + assert (response, answered) == (delivery.UNDELIVERED_DECLINED, False) + assert adapter.sent_text == [] + + # --- Atom: no chat surface at all ----------------------------------------------------------- diff --git a/website/docs/user-guide/messaging/telegram.md b/website/docs/user-guide/messaging/telegram.md index f558f49e5a..e15fb677e3 100644 --- a/website/docs/user-guide/messaging/telegram.md +++ b/website/docs/user-guide/messaging/telegram.md @@ -1326,7 +1326,7 @@ When the agent calls the `clarify` tool — to ask which approach you prefer, ge Tap a button to answer, or tap **Other** to type a free-form response (the next message you send becomes the answer). Open-ended `clarify` calls (no preset choices) skip the buttons and just capture your next message. -Configure the response timeout via `agent.clarify_timeout` in `~/.hermes/config.yaml` (default `600` seconds). If you don't respond within the timeout, the agent unblocks with a sentinel message and adapts rather than hanging. +Configure the response timeout via `agent.clarify_timeout` in `~/.hermes/config.yaml` (default `3600` seconds). If you don't respond within the timeout, the agent unblocks with a sentinel message and adapts rather than hanging. If Telegram cannot render the button card (the Bot API rejects it, or the send fails after its 15-second acknowledgement window), Hermes re-asks the same question as a plain numbered-list message and your typed reply (a number or the option text) is taken as the answer. When even that cannot be delivered, the agent is released at once with `[clarify prompt could not be delivered]` instead of waiting out the timeout and mistaking the silence for you not answering.