fix(clarify): one canonical timeout across CLI, TUI/desktop, and gateway (#69774)

* 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>

cli.py

@@ -11533,7 +11533,11 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
         """
         import time as _time
 
-        timeout = CLI_CONFIG.get("clarify", {}).get("timeout", 120)
+        from tools.clarify_gateway import resolve_clarify_timeout
+
+        # Canonical clarify timeout, shared with the gateway/TUI path. `<= 0`
+        # means unlimited (never auto-skip mid-think) → a null deadline.
+        timeout = resolve_clarify_timeout(CLI_CONFIG)
         response_queue = queue.Queue()
         is_open_ended = not choices
 
@@ -11543,7 +11547,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
             "selected": 0,
             "response_queue": response_queue,
         }
-        self._clarify_deadline = _time.monotonic() + timeout
+        self._clarify_deadline = None if timeout <= 0 else _time.monotonic() + timeout
         # Open-ended questions skip straight to freetext input
         self._clarify_freetext = is_open_ended
 
@@ -11560,13 +11564,15 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
         while True:
             try:
                 result = response_queue.get(timeout=1)
-                self._clarify_deadline = 0
+                self._clarify_deadline = None
                 self._persist_prompt_summary("?", "Clarify", question, str(result))
                 return result
             except queue.Empty:
-                remaining = self._clarify_deadline - _time.monotonic()
-                if remaining <= 0:
-                    break
+                # None deadline = unlimited: never auto-skip, just keep polling.
+                if self._clarify_deadline is not None:
+                    remaining = self._clarify_deadline - _time.monotonic()
+                    if remaining <= 0:
+                        break
                 now = _time.monotonic()
                 if now - _last_countdown_refresh >= 1.0:
                     _last_countdown_refresh = now
@@ -11575,7 +11581,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
         # Timed out — tear down the UI and let the agent decide
         self._clarify_state = None
         self._clarify_freetext = False
-        self._clarify_deadline = 0
+        self._clarify_deadline = None
         self._paint_now()
         _cprint(f"\n{_DIM}(clarify timed out after {timeout}s — agent will decide){_RST}")
         return (
@@ -14583,8 +14589,12 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin):
                 ]
 
             if cli_ref._clarify_state:
-                remaining = max(0, int(cli_ref._clarify_deadline - time.monotonic()))
-                countdown = f'  ({remaining}s)' if cli_ref._clarify_deadline else ''
+                # None deadline = unlimited wait → hide the countdown entirely.
+                if cli_ref._clarify_deadline is None:
+                    countdown = ''
+                else:
+                    remaining = max(0, int(cli_ref._clarify_deadline - time.monotonic()))
+                    countdown = f'  ({remaining}s)'
                 if cli_ref._clarify_freetext:
                     return [
                         ('class:hint', '  type your answer and press Enter'),

hermes_cli/callbacks.py

@@ -22,8 +22,11 @@ def clarify_callback(cli, question, choices):
     responds. Returns the user's choice or a timeout message.
     """
     from cli import CLI_CONFIG
+    from tools.clarify_gateway import resolve_clarify_timeout
 
-    timeout = CLI_CONFIG.get("clarify", {}).get("timeout", 120)
+    # Canonical clarify timeout, shared with the gateway/TUI path. `<= 0`
+    # means unlimited (never auto-skip mid-think) → a null deadline.
+    timeout = resolve_clarify_timeout(CLI_CONFIG)
     response_queue = queue.Queue()
     is_open_ended = not choices
 
@@ -33,7 +36,7 @@ def clarify_callback(cli, question, choices):
         "selected": 0,
         "response_queue": response_queue,
     }
-    cli._clarify_deadline = _time.monotonic() + timeout
+    cli._clarify_deadline = None if timeout <= 0 else _time.monotonic() + timeout
     cli._clarify_freetext = is_open_ended
 
     if hasattr(cli, "_app") and cli._app:
@@ -42,18 +45,20 @@ def clarify_callback(cli, question, choices):
     while True:
         try:
             result = response_queue.get(timeout=1)
-            cli._clarify_deadline = 0
+            cli._clarify_deadline = None
             return result
         except queue.Empty:
-            remaining = cli._clarify_deadline - _time.monotonic()
-            if remaining <= 0:
-                break
+            # None deadline = unlimited: never auto-skip, just keep polling.
+            if cli._clarify_deadline is not None:
+                remaining = cli._clarify_deadline - _time.monotonic()
+                if remaining <= 0:
+                    break
             if hasattr(cli, "_app") and cli._app:
                 cli._app.invalidate()
 
     cli._clarify_state = None
     cli._clarify_freetext = False
-    cli._clarify_deadline = 0
+    cli._clarify_deadline = None
     if hasattr(cli, "_app") and cli._app:
         cli._app.invalidate()
     cprint(f"\n{_DIM}(clarify timed out after {timeout}s — agent will decide){_RST}")

tests/test_tui_gateway_server.py

@@ -12110,3 +12110,36 @@ def test_tts_stream_vad_barge_in_cuts_pipeline_and_submits_capture(monkeypatch,
     assert not wav.exists()  # capture temp file cleaned up
     assert ts.take_speech_interrupted() is True  # VAD cut latches the model note
     server._tts_stream_stop()
+
+
+def test_clarify_callback_uses_configured_timeout(monkeypatch):
+    """The TUI/desktop clarify bridge honors the canonical clarify timeout
+    (via _clarify_timeout_seconds) instead of the hardcoded _block default."""
+    captured = {}
+
+    monkeypatch.setattr(server, "_clarify_timeout_seconds", lambda: 42)
+
+    def fake_block(event, sid, payload, timeout=300):
+        captured.update(event=event, sid=sid, payload=payload, timeout=timeout)
+        return "answer"
+
+    monkeypatch.setattr(server, "_block", fake_block)
+
+    result = server._agent_cbs("sid-1")["clarify_callback"]("Pick one", ["a", "b"])
+
+    assert result == "answer"
+    assert captured["event"] == "clarify.request"
+    assert captured["timeout"] == 42
+    assert captured["payload"] == {"question": "Pick one", "choices": ["a", "b"]}
+
+
+@pytest.mark.parametrize(
+    ("configured", "expected"),
+    [(0, None), (-1, None), (42, 42)],
+)
+def test_clarify_timeout_seconds_maps_non_positive_to_unlimited(monkeypatch, configured, expected):
+    """A ``<= 0`` clarify timeout means unlimited and reaches _block as None
+    (ev.wait(None) waits forever) rather than an immediate ev.wait(0) skip."""
+    monkeypatch.setattr("tools.clarify_gateway.get_clarify_timeout", lambda: configured)
+
+    assert server._clarify_timeout_seconds() == expected

tests/tools/test_clarify_gateway.py

@@ -294,3 +294,148 @@ class TestGatewayTextIntercept:
         
         # Clean up
         cm.clear_session("sk-tf")
+
+
+class TestCoverageGaps:
+    """Cover remaining branches: signature(), get_entry miss, find_awaiting
+    with deleted entry, cancel with None entry, timeout exception, get_notify."""
+
+    def setup_method(self):
+        _clear_clarify_state()
+
+    def test_entry_signature(self):
+        """_ClarifyEntry.signature() returns the expected dict."""
+        from tools import clarify_gateway as cm
+
+        entry = cm.register("sig1", "sk", "Q?", ["A", "B"])
+        sig = entry.signature()
+        assert sig["clarify_id"] == "sig1"
+        assert sig["session_key"] == "sk"
+        assert sig["question"] == "Q?"
+        assert sig["choices"] == ["A", "B"]
+
+    def test_entry_signature_no_choices(self):
+        """signature() returns None for choices when open-ended."""
+        from tools import clarify_gateway as cm
+
+        entry = cm.register("sig2", "sk", "Q?", None)
+        sig = entry.signature()
+        assert sig["choices"] is None
+
+    def test_wait_for_response_unknown_id_returns_none(self):
+        """wait_for_response on a non-existent id returns None immediately."""
+        from tools import clarify_gateway as cm
+
+        assert cm.wait_for_response("nonexistent-id", timeout=0.1) is None
+
+    def test_find_awaiting_skips_deleted_entry(self):
+        """get_pending_for_session skips entries that were removed from _entries
+        but still listed in _session_index."""
+        from tools import clarify_gateway as cm
+
+        cm.register("a1", "sk", "Q?", None)
+        # Manually remove from _entries but leave in _session_index
+        with cm._lock:
+            cm._entries.pop("a1", None)
+        # No entry to find → returns None
+        assert cm.get_pending_for_session("sk") is None
+
+    def test_clear_session_skips_deleted_entry(self):
+        """clear_session skips entries that are None (already removed)."""
+        from tools import clarify_gateway as cm
+
+        cm.register("c1", "sk", "Q?", ["A"])
+        # Manually remove from _entries but leave in _session_index
+        with cm._lock:
+            cm._entries.pop("c1", None)
+        # Should return 0 cancelled (entry was already gone)
+        cancelled = cm.clear_session("sk")
+        assert cancelled == 0
+
+    def test_get_clarify_timeout_exception_returns_default(self, monkeypatch):
+        """get_clarify_timeout returns 3600 when load_config raises."""
+        from tools import clarify_gateway as cm
+
+        monkeypatch.setattr("hermes_cli.config.load_config",
+                            lambda: (_ for _ in ()).throw(RuntimeError("boom")))
+        assert cm.get_clarify_timeout() == 3600
+
+    def test_get_notify_returns_callback(self):
+        """get_notify returns the registered callback."""
+        from tools import clarify_gateway as cm
+
+        cb = lambda entry: None
+        cm.register_notify("sk-notify", cb)
+        assert cm.get_notify("sk-notify") is cb
+
+    def test_get_notify_returns_none_when_not_registered(self):
+        """get_notify returns None for an unregistered session."""
+        from tools import clarify_gateway as cm
+
+        assert cm.get_notify("unregistered") is None
+
+
+class TestClarifyTimeoutResolution:
+    """resolve_clarify_timeout is the single source of truth for the clarify
+    timeout, shared by the CLI, TUI/desktop, and messaging-gateway paths."""
+
+    def test_canonical_agent_key(self):
+        from tools import clarify_gateway as cm
+
+        assert cm.resolve_clarify_timeout({"agent": {"clarify_timeout": 900}}) == 900
+
+    def test_legacy_clarify_key_overrides(self):
+        """An explicitly-set legacy top-level clarify.timeout wins, for
+        back-compat with users who set it before agent.clarify_timeout existed."""
+        from tools import clarify_gateway as cm
+
+        cfg = {"clarify": {"timeout": 42}, "agent": {"clarify_timeout": 900}}
+        assert cm.resolve_clarify_timeout(cfg) == 42
+
+    def test_default_when_unset(self):
+        from tools import clarify_gateway as cm
+
+        assert cm.resolve_clarify_timeout({}) == 3600
+
+    def test_non_numeric_falls_back_to_default(self):
+        from tools import clarify_gateway as cm
+
+        assert cm.resolve_clarify_timeout({"agent": {"clarify_timeout": "nope"}}) == 3600
+
+    def test_non_positive_preserved_as_unlimited_sentinel(self):
+        """<= 0 is passed through verbatim — the waiting loops read it as
+        'unlimited', so the resolver must not clamp it to a positive default."""
+        from tools import clarify_gateway as cm
+
+        assert cm.resolve_clarify_timeout({"agent": {"clarify_timeout": 0}}) == 0
+        assert cm.resolve_clarify_timeout({"clarify": {"timeout": -1}}) == -1
+
+
+class TestUnlimitedWait:
+    """timeout <= 0 makes wait_for_response block until the answer arrives
+    instead of auto-skipping."""
+
+    def setup_method(self):
+        _clear_clarify_state()
+
+    def test_zero_timeout_waits_until_resolved(self):
+        from tools import clarify_gateway as cm
+
+        cm.register("u1", "sk", "Q?", ["A", "B"])
+        result_box = {}
+
+        def waiter():
+            result_box["r"] = cm.wait_for_response("u1", timeout=0)
+
+        t = threading.Thread(target=waiter)
+        t.start()
+        # An unlimited wait cannot finish while nothing resolves it: still
+        # running after a comfortable margin (old code auto-skipped at once).
+        t.join(timeout=1.5)
+        assert t.is_alive()
+
+        # Once resolved, the unlimited wait returns the real answer.
+        cm.resolve_gateway_clarify("u1", "B")
+        t.join(timeout=5.0)
+        assert not t.is_alive()
+        assert result_box["r"] == "B"

tools/clarify_gateway.py

@@ -108,6 +108,10 @@ def wait_for_response(clarify_id: str, timeout: float) -> Optional[str]:
     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:
@@ -120,13 +124,19 @@ def wait_for_response(clarify_id: str, timeout: float) -> Optional[str]:
     except Exception:  # pragma: no cover - optional
         touch_activity_if_due = None
 
-    deadline = time.monotonic() + max(timeout, 0.0)
+    # 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:
-        remaining = deadline - time.monotonic()
-        if remaining <= 0:
-            break
-        if entry.event.wait(timeout=min(1.0, remaining)):
+        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")
@@ -300,6 +310,29 @@ def clear_session(session_key: str) -> int:
 # 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.
 
@@ -311,13 +344,14 @@ def get_clarify_timeout() -> int:
     tap landed on a dead entry and the agent hung on ``running: clarify``
     (#32762).
 
-    Reads ``agent.clarify_timeout`` from config.yaml.
+    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
-        cfg = load_config() or {}
-        agent_cfg = cfg.get("agent", {}) or {}
-        return int(agent_cfg.get("clarify_timeout", 3600))
+        return resolve_clarify_timeout(load_config() or {})
     except Exception:
         return 3600
 

tui_gateway/server.py

@@ -2411,7 +2411,7 @@ def _enable_gateway_prompts() -> None:
 # ── Blocking prompt factory ──────────────────────────────────────────
 
 
-def _block(event: str, sid: str, payload: dict, timeout: int = 300) -> str:
+def _block(event: str, sid: str, payload: dict, timeout: float | None = 300) -> str:
     rid = uuid.uuid4().hex[:8]
     ev = threading.Event()
     with _prompt_lock:
@@ -2423,7 +2423,10 @@ def _block(event: str, sid: str, payload: dict, timeout: int = 300) -> str:
     answer_present = False
     try:
         _emit(event, sid, payload)
-        answered = ev.wait(timeout=timeout)
+        # Natural Event semantics: None → wait forever (clarify configured with
+        # clarify_timeout <= 0, released only by a real answer or
+        # session.interrupt), 0 → return immediately, > 0 → bounded wait.
+        answered = ev.wait(timeout)
     finally:
         with _prompt_lock:
             _pending.pop(rid, None)
@@ -2452,6 +2455,19 @@ def _block(event: str, sid: str, payload: dict, timeout: int = 300) -> str:
     return answer
 
 
+def _clarify_timeout_seconds() -> float | None:
+    """Clarify wait (seconds) for the TUI/desktop bridge, from the same
+    canonical config the messaging gateway and CLI use. Falls back to the
+    historical 300s _block default if config can't be read. ``<= 0`` in config
+    means unlimited and is returned as ``None`` (never auto-skip)."""
+    try:
+        from tools.clarify_gateway import get_clarify_timeout
+        timeout = get_clarify_timeout()
+        return timeout if timeout > 0 else None
+    except Exception:
+        return 300
+
+
 def _clear_pending(sid: str | None = None) -> None:
     """Release pending prompts with an empty answer.
 
@@ -4531,7 +4547,10 @@ def _agent_cbs(sid: str) -> dict:
             "notification.clear", sid, {"key": key}
         ),
         "clarify_callback": lambda q, c: _block(
-            "clarify.request", sid, {"question": q, "choices": c}
+            "clarify.request",
+            sid,
+            {"question": q, "choices": c},
+            timeout=_clarify_timeout_seconds(),
         ),
         # read_terminal tool (desktop GUI): same blocking bridge as clarify — the
         # renderer answers terminal.read.respond with the serialized buffer.