mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
* test(clarify-gateway): cover signature, timeout fallback, and notify paths for 100% coverage
Fixes #36531
(cherry picked from commit 5265dfe2f5)
* fix(clarify): one canonical timeout across CLI, TUI/desktop, and gateway
The clarify wait timeout was resolved three different (wrong) ways:
- CLI (`cli.py`, `hermes_cli/callbacks.py`) read a non-existent top-level
`clarify.timeout`, so it always fell through to a hardcoded 120s instead of
the canonical `agent.clarify_timeout` (default 3600) the gateway uses (#42969).
- The TUI/desktop bridge called `_block("clarify.request", …)` with no timeout,
so it used the hardcoded 300s `_block` default and ignored config (#51960).
- There was no way to disable the auto-skip: a user who wanted the agent to wait
indefinitely while they think couldn't get it.
Collapse all of this onto a single resolver:
- `tools.clarify_gateway.resolve_clarify_timeout(config)` is the one source of
truth. Order: explicit legacy `clarify.timeout` (back-compat) → canonical
`agent.clarify_timeout` → 3600. `<= 0` is preserved verbatim as "unlimited".
- CLI, callbacks, and the TUI bridge (`_clarify_timeout_seconds`) all route
through it, so the three surfaces can't drift.
- `<= 0` means unlimited everywhere: `wait_for_response` and `_block` drop the
deadline (heartbeat still fires), and the CLI hides its countdown.
Tests: resolver order / default / non-numeric / unlimited-sentinel; an
unlimited `wait_for_response` blocks until resolved rather than auto-skipping;
the TUI clarify bridge passes the configured timeout to `_block`.
Supersedes #42974 (CLI key), #51993 (TUI honors config), and #68986 (unlimited
wait); folds in #52031 (clarify_gateway coverage).
Co-authored-by: liuhao1024 <liuhao1024@users.noreply.github.com>
Co-authored-by: lkevincc0 <lkevincc0@users.noreply.github.com>
Co-authored-by: theone139344 <theone139344@users.noreply.github.com>
Co-authored-by: baauzi <baauzi@users.noreply.github.com>
---------
Co-authored-by: Christopher-Schulze <210261288+Christopher-Schulze@users.noreply.github.com>
Co-authored-by: liuhao1024 <liuhao1024@users.noreply.github.com>
Co-authored-by: lkevincc0 <lkevincc0@users.noreply.github.com>
Co-authored-by: theone139344 <theone139344@users.noreply.github.com>
Co-authored-by: baauzi <baauzi@users.noreply.github.com>
387 lines
14 KiB
Python
387 lines
14 KiB
Python
"""Gateway-side clarify primitive (blocking event-based queue).
|
|
|
|
The ``clarify`` tool needs to ask the user a question and block the agent
|
|
thread until they respond. In CLI mode this is trivial — ``input()`` is
|
|
synchronous. In gateway mode the agent runs on a worker thread while the
|
|
event loop handles the user's reply, so we need a thread-safe primitive
|
|
that:
|
|
|
|
* stores a pending clarify request (with a generated ``clarify_id``),
|
|
* blocks the agent thread on an ``Event``,
|
|
* resolves the wait when the gateway's button-callback or text-intercept
|
|
fires ``resolve_gateway_clarify(clarify_id, response)``,
|
|
* supports timeouts so a user who never responds does NOT hang the agent
|
|
thread forever (which would also pin the gateway's running-agent guard).
|
|
|
|
State is module-level (same shape as ``tools.approval``) so platform
|
|
adapters can call ``resolve_gateway_clarify`` without holding a back-
|
|
reference to the ``GatewayRunner`` instance.
|
|
|
|
Two delivery paths from the adapter:
|
|
|
|
1. **Button UI** — adapters override ``send_clarify`` to render inline
|
|
buttons (e.g. Telegram ``InlineKeyboardMarkup``). The button
|
|
callback resolves with the chosen string. A final "Other (type
|
|
answer)" button enters text-capture mode for free-form responses.
|
|
|
|
2. **Text fallback** — adapters without rich UI render a numbered list.
|
|
The user replies with a number ("2") or with free text; the gateway's
|
|
``_handle_message`` intercepts the reply and resolves directly.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
import threading
|
|
import time
|
|
from dataclasses import dataclass, field
|
|
from typing import Callable, Dict, List, Optional
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
|
|
# =========================================================================
|
|
# Module-level state
|
|
# =========================================================================
|
|
|
|
@dataclass
|
|
class _ClarifyEntry:
|
|
"""One pending clarify request inside a gateway session."""
|
|
clarify_id: str
|
|
session_key: str
|
|
question: str
|
|
choices: Optional[List[str]]
|
|
event: threading.Event = field(default_factory=threading.Event)
|
|
response: Optional[str] = None
|
|
awaiting_text: bool = False # set when user picked "Other" or clarify is open-ended
|
|
|
|
def signature(self) -> Dict[str, object]:
|
|
return {
|
|
"clarify_id": self.clarify_id,
|
|
"session_key": self.session_key,
|
|
"question": self.question,
|
|
"choices": list(self.choices) if self.choices else None,
|
|
}
|
|
|
|
|
|
_lock = threading.RLock()
|
|
# clarify_id → _ClarifyEntry (primary lookup for button callbacks)
|
|
_entries: Dict[str, _ClarifyEntry] = {}
|
|
# session_key → list[clarify_id] (FIFO; for text-fallback intercept and session cleanup)
|
|
_session_index: Dict[str, List[str]] = {}
|
|
|
|
|
|
# =========================================================================
|
|
# Public API — agent-thread side
|
|
# =========================================================================
|
|
|
|
def register(
|
|
clarify_id: str,
|
|
session_key: str,
|
|
question: str,
|
|
choices: Optional[List[str]],
|
|
) -> _ClarifyEntry:
|
|
"""Register a pending clarify request and return the entry.
|
|
|
|
The caller (gateway clarify_callback) will then send the prompt to the
|
|
user and block on ``wait_for_response(clarify_id, timeout)``.
|
|
"""
|
|
entry = _ClarifyEntry(
|
|
clarify_id=clarify_id,
|
|
session_key=session_key,
|
|
question=question,
|
|
choices=list(choices) if choices else None,
|
|
# Open-ended (no choices) → next message IS the response, no buttons needed.
|
|
awaiting_text=not bool(choices),
|
|
)
|
|
with _lock:
|
|
_entries[clarify_id] = entry
|
|
_session_index.setdefault(session_key, []).append(clarify_id)
|
|
return entry
|
|
|
|
|
|
def wait_for_response(clarify_id: str, timeout: float) -> Optional[str]:
|
|
"""Block on the entry's event until resolved or timeout fires.
|
|
|
|
Polls in 1-second slices so the agent's inactivity heartbeat keeps
|
|
firing — without this, ``Event.wait(timeout=600)`` blocks the thread
|
|
for 10 minutes with zero activity touches and the gateway's inactivity
|
|
watchdog kills the agent while the user is still typing.
|
|
|
|
``timeout <= 0`` means an unlimited wait (never auto-skip mid-think); the
|
|
heartbeat still fires each slice so inactivity watchdogs don't kill a live
|
|
prompt.
|
|
|
|
Returns the resolved response string, or ``None`` on timeout.
|
|
"""
|
|
with _lock:
|
|
entry = _entries.get(clarify_id)
|
|
if entry is None:
|
|
return None
|
|
|
|
try:
|
|
from tools.environments.base import touch_activity_if_due
|
|
except Exception: # pragma: no cover - optional
|
|
touch_activity_if_due = None
|
|
|
|
# 0 / negative → unlimited: no deadline, poll forever in 1s slices.
|
|
unlimited = timeout is None or float(timeout) <= 0.0
|
|
deadline = None if unlimited else time.monotonic() + float(timeout)
|
|
activity_state = {"last_touch": time.monotonic(), "start": time.monotonic()}
|
|
while True:
|
|
if deadline is None:
|
|
slice_s = 1.0
|
|
else:
|
|
remaining = deadline - time.monotonic()
|
|
if remaining <= 0:
|
|
break
|
|
slice_s = min(1.0, remaining)
|
|
if entry.event.wait(timeout=slice_s):
|
|
break
|
|
if touch_activity_if_due is not None:
|
|
touch_activity_if_due(activity_state, "waiting for user clarify response")
|
|
|
|
with _lock:
|
|
# Remove from indices regardless of resolution outcome.
|
|
_entries.pop(clarify_id, None)
|
|
ids = _session_index.get(entry.session_key)
|
|
if ids and clarify_id in ids:
|
|
ids.remove(clarify_id)
|
|
if not ids:
|
|
_session_index.pop(entry.session_key, None)
|
|
|
|
return entry.response
|
|
|
|
|
|
# =========================================================================
|
|
# Public API — gateway / adapter side
|
|
# =========================================================================
|
|
|
|
def resolve_gateway_clarify(clarify_id: str, response: str) -> bool:
|
|
"""Unblock the agent thread waiting on ``clarify_id``.
|
|
|
|
Returns True if an entry was found and resolved, False otherwise
|
|
(already resolved, expired, or never existed).
|
|
"""
|
|
with _lock:
|
|
entry = _entries.get(clarify_id)
|
|
if entry is None:
|
|
return False
|
|
entry.response = str(response) if response is not None else ""
|
|
entry.event.set()
|
|
return True
|
|
|
|
|
|
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.
|
|
|
|
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 []
|
|
for cid in ids:
|
|
entry = _entries.get(cid)
|
|
if entry is None:
|
|
continue
|
|
if include_choice_prompts or entry.awaiting_text:
|
|
return entry
|
|
return None
|
|
|
|
|
|
def _coerce_text_response(entry: _ClarifyEntry, response: str) -> Optional[str]:
|
|
"""Map typed choice replies to canonical choice text, otherwise keep or reject custom text.
|
|
|
|
For native interactive multi-choice clarifies (button UI, awaiting_text=False):
|
|
- Accept numeric selections ("2" → choice[1])
|
|
- Accept exact choice label matches (case-insensitive)
|
|
- Reject arbitrary prose (return None) so the message continues as a normal turn
|
|
|
|
For text fallback or awaiting_text mode:
|
|
- Accept any text (numeric/label/custom) after passing through coercion
|
|
|
|
For open-ended clarifies (no choices):
|
|
- Accept any text
|
|
|
|
Returns None when the response should be rejected (arbitrary prose for native multi-choice).
|
|
"""
|
|
text = str(response).strip()
|
|
|
|
if not entry.choices:
|
|
# Open-ended: accept any text
|
|
return text
|
|
|
|
# Try numeric selection first (always valid for multi-choice)
|
|
try:
|
|
idx = int(text) - 1
|
|
except ValueError:
|
|
idx = -1
|
|
|
|
if 0 <= idx < len(entry.choices):
|
|
return entry.choices[idx]
|
|
|
|
# Try exact choice label match (always valid for multi-choice)
|
|
for choice in entry.choices:
|
|
if text.casefold() == str(choice).strip().casefold():
|
|
return str(choice).strip()
|
|
|
|
# For text fallback or awaiting_text mode, accept custom text
|
|
# For native interactive multi-choice mode, reject arbitrary prose
|
|
if entry.awaiting_text:
|
|
return text
|
|
|
|
return None
|
|
|
|
|
|
def resolve_text_response_for_session(session_key: str, response: str) -> bool:
|
|
"""Resolve the oldest pending clarify in ``session_key`` from typed text.
|
|
|
|
Returns False if no pending clarify exists or if the response was rejected
|
|
(arbitrary prose for native interactive multi-choice clarifies).
|
|
"""
|
|
entry = get_pending_for_session(session_key, include_choice_prompts=True)
|
|
if entry is None:
|
|
return False
|
|
|
|
coerced = _coerce_text_response(entry, response)
|
|
if coerced is None:
|
|
# Response rejected: message should continue as a normal turn
|
|
return False
|
|
|
|
return resolve_gateway_clarify(
|
|
entry.clarify_id,
|
|
coerced,
|
|
)
|
|
|
|
|
|
def mark_awaiting_text(clarify_id: str) -> bool:
|
|
"""Flip an entry into text-capture mode (user picked the 'Other' button).
|
|
|
|
Returns True if the entry exists and was flipped, False otherwise.
|
|
"""
|
|
with _lock:
|
|
entry = _entries.get(clarify_id)
|
|
if entry is None:
|
|
return False
|
|
entry.awaiting_text = True
|
|
return True
|
|
|
|
|
|
def has_pending(session_key: str) -> bool:
|
|
"""Return True when this session has at least one pending clarify entry."""
|
|
with _lock:
|
|
ids = _session_index.get(session_key) or []
|
|
return any(_entries.get(cid) is not None for cid in ids)
|
|
|
|
|
|
def clear_session(session_key: str) -> int:
|
|
"""Resolve and drop every pending clarify for a session.
|
|
|
|
Used by session-boundary cleanup (e.g. ``/new``, gateway shutdown,
|
|
cached-agent eviction) so blocked agent threads don't hang past the
|
|
end of their session. Returns the number of entries cancelled.
|
|
"""
|
|
with _lock:
|
|
ids = list(_session_index.pop(session_key, []) or [])
|
|
entries = [_entries.pop(cid, None) for cid in ids]
|
|
cancelled = 0
|
|
for entry in entries:
|
|
if entry is None:
|
|
continue
|
|
# Empty string sentinel — agent code can distinguish from a real
|
|
# response by inspecting the wait_for_response return value
|
|
# alongside its own timeout deadline. Most callers just treat any
|
|
# falsy result as "user did not respond".
|
|
entry.response = ""
|
|
entry.event.set()
|
|
cancelled += 1
|
|
return cancelled
|
|
|
|
|
|
# =========================================================================
|
|
# Config
|
|
# =========================================================================
|
|
|
|
def resolve_clarify_timeout(config: dict) -> int:
|
|
"""Resolve the clarify timeout (seconds) from an already-loaded config dict.
|
|
|
|
Single source of truth shared by every surface (messaging gateway, CLI,
|
|
TUI/desktop) so the timeout can't drift between them. Resolution order:
|
|
|
|
1. legacy top-level ``clarify.timeout`` if a user explicitly set it,
|
|
2. else the canonical ``agent.clarify_timeout``,
|
|
3. else 3600 (1 hour).
|
|
|
|
``<= 0`` is preserved verbatim and means *unlimited* to callers (never
|
|
auto-skip while the user is still deciding); the waiting loops translate
|
|
that into a null deadline. A non-numeric value falls back to 3600.
|
|
"""
|
|
raw = (config.get("clarify") or {}).get("timeout")
|
|
if raw is None:
|
|
raw = (config.get("agent") or {}).get("clarify_timeout", 3600)
|
|
try:
|
|
return int(raw)
|
|
except (TypeError, ValueError):
|
|
return 3600
|
|
|
|
|
|
def get_clarify_timeout() -> int:
|
|
"""Read the clarify response timeout (seconds) from config.
|
|
|
|
Defaults to 3600 (1 hour) — long enough that a user who steps away
|
|
(meeting, AFK, slow to read) still finds a live entry when they tap
|
|
the button, short enough that a genuinely abandoned prompt eventually
|
|
unblocks the agent thread instead of pinning the running-agent guard
|
|
forever. The old 600s default evicted the entry mid-think, so a late
|
|
tap landed on a dead entry and the agent hung on ``running: clarify``
|
|
(#32762).
|
|
|
|
Reads ``agent.clarify_timeout`` from config.yaml (see
|
|
:func:`resolve_clarify_timeout` for the full resolution order). Set to
|
|
``0`` (or negative) for an unlimited wait — never auto-skip while the user
|
|
is still deciding.
|
|
"""
|
|
try:
|
|
from hermes_cli.config import load_config
|
|
return resolve_clarify_timeout(load_config() or {})
|
|
except Exception:
|
|
return 3600
|
|
|
|
|
|
# =========================================================================
|
|
# Per-session notify hook (gateway → adapter bridge)
|
|
# =========================================================================
|
|
# Mirrors tools.approval's _gateway_notify_cbs: the gateway registers a
|
|
# per-session callback that sends the clarify prompt to the user. The
|
|
# callback bridges sync→async (runs on the agent thread; schedules the
|
|
# adapter ``send_clarify`` call on the event loop).
|
|
|
|
_notify_cbs: Dict[str, Callable[[_ClarifyEntry], None]] = {}
|
|
|
|
|
|
def register_notify(session_key: str, cb: Callable[[_ClarifyEntry], None]) -> None:
|
|
"""Register a per-session notify callback used by ``clarify_callback``."""
|
|
with _lock:
|
|
_notify_cbs[session_key] = cb
|
|
|
|
|
|
def unregister_notify(session_key: str) -> None:
|
|
"""Drop the per-session notify callback and cancel any pending clarify entries."""
|
|
with _lock:
|
|
_notify_cbs.pop(session_key, None)
|
|
# Cancel any pending entries so blocked threads unwind when the run
|
|
# ends (interrupt, completion, gateway shutdown).
|
|
clear_session(session_key)
|
|
|
|
|
|
def get_notify(session_key: str) -> Optional[Callable[[_ClarifyEntry], None]]:
|
|
with _lock:
|
|
return _notify_cbs.get(session_key)
|