fix(discord): release the agent when clarify buttons expire

plugins/platforms/discord/adapter.py

@@ -844,6 +844,19 @@ def _read_dm_role_auth_guild() -> Optional[int]:
 _DISCORD_PROMPT_TIMEOUT_MIN = 30
 _DISCORD_PROMPT_TIMEOUT_MAX = 900
 
+# Grace window (seconds) granted after a clarify's BUTTONS expire, during
+# which a typed reply still resolves the prompt. Keep it strictly shorter
+# than ``agent.clarify_timeout`` minus the view timeout, otherwise the
+# agent-side wait fires first and the window is dead time. 0 disables the
+# window: an expired view releases the agent immediately.
+_CLARIFY_TEXT_GRACE_DEFAULT = 300
+_CLARIFY_TEXT_GRACE_MAX = 3600
+
+# Strong references to in-flight clarify-expiry tasks. asyncio only keeps a
+# weak reference to a running task, so without this the GC can collect one
+# mid-sleep and the agent never gets released.
+_CLARIFY_EXPIRY_TASKS: set = set()
+
 
 def _env_bool(name: str, default: bool = False) -> bool:
     raw = os.getenv(name, "").strip().lower()
@@ -883,6 +896,44 @@ def _read_discord_prompt_timeout() -> int:
     return seconds
 
 
+def _read_clarify_text_grace() -> int:
+    """Return the typed-answer grace window (seconds) after buttons expire.
+
+    Reads ``approvals.discord_clarify_text_grace`` from config.yaml, falling
+    back to ``_CLARIFY_TEXT_GRACE_DEFAULT``. Clamped to
+    ``[0, _CLARIFY_TEXT_GRACE_MAX]``; 0 means "release the agent as soon as
+    the view times out".
+    """
+    raw: Any = None
+    try:
+        from hermes_cli.config import read_raw_config
+        cfg = read_raw_config() or {}
+        approvals_cfg = cfg.get("approvals", {}) or {}
+        raw = approvals_cfg.get("discord_clarify_text_grace")
+    except Exception:
+        return _CLARIFY_TEXT_GRACE_DEFAULT
+    if raw is None or raw == "":
+        return _CLARIFY_TEXT_GRACE_DEFAULT
+    try:
+        seconds = int(raw)
+    except (TypeError, ValueError):
+        return _CLARIFY_TEXT_GRACE_DEFAULT
+    if seconds < 0:
+        return 0
+    if seconds > _CLARIFY_TEXT_GRACE_MAX:
+        return _CLARIFY_TEXT_GRACE_MAX
+    return seconds
+
+
+def _clarify_entry_pending(clarify_id: str) -> bool:
+    """True while ``clarify_id`` is still waiting for an answer."""
+    try:
+        from tools.clarify_gateway import _entries as _clarify_entries  # type: ignore
+        return _clarify_entries.get(clarify_id) is not None
+    except Exception:
+        return False
+
+
 class DiscordAdapter(BasePlatformAdapter):
     """
     Discord bot adapter.
@@ -9128,21 +9179,101 @@ async def _on_other(self, interaction: "discord.Interaction") -> None:
                 except Exception:
                     pass
 
+        async def _edit_expired_embed(self, footer: str, color) -> None:
+            """Repaint the prompt message with an expiry footer."""
+            msg = getattr(self, "_message", None)
+            if not msg:
+                return
+            try:
+                embed = msg.embeds[0] if msg.embeds else None
+                if embed:
+                    embed.color = color
+                    embed.set_footer(text=footer)
+                await msg.edit(embed=embed, view=self)
+            except Exception:
+                pass
+
+        async def _resolve_after_grace(self, grace: int) -> None:
+            """Unblock the agent once the typed-answer grace window closes.
+
+            The view's timeout only kills the BUTTONS.  The agent thread is
+            still parked in ``clarify_gateway.wait_for_response`` until its
+            own ``agent.clarify_timeout`` fires — an hour by default, and
+            never when that is set to 0.  Without this the session stays
+            pinned behind a prompt the user can no longer answer by
+            clicking, and every follow-up message queues up behind a turn
+            that cannot finish.
+            """
+            try:
+                if grace > 0:
+                    await asyncio.sleep(grace)
+                if not _clarify_entry_pending(self.clarify_id):
+                    return  # user typed an answer during the grace window
+                from tools.clarify_gateway import resolve_gateway_clarify
+                resolved = resolve_gateway_clarify(self.clarify_id, "")
+                logger.info(
+                    "Discord clarify expired unanswered (id=%s, grace=%ds, ok=%s) — "
+                    "released the agent with an empty response",
+                    self.clarify_id, grace, resolved,
+                )
+                await self._edit_expired_embed(
+                    "⏱ Prompt expired — no action taken",
+                    discord.Color.greyple(),
+                )
+            except asyncio.CancelledError:
+                raise
+            except Exception as exc:
+                logger.warning(
+                    "Discord clarify expiry release failed (id=%s): %s",
+                    self.clarify_id, exc,
+                )
+
         async def on_timeout(self):
             self.resolved = True
             for child in self.children:
                 child.disabled = True
-            # Visually update the Discord message so buttons appear disabled.
-            msg = getattr(self, '_message', None)
-            if msg:
-                try:
-                    embed = msg.embeds[0] if msg.embeds else None
-                    if embed:
-                        embed.color = discord.Color.greyple()
-                        embed.set_footer(text="⏱ Prompt expired — no action taken")
-                    await msg.edit(embed=embed, view=self)
-                except Exception:
-                    pass
+
+            # The entry is already gone when the agent moved on by itself
+            # (answered elsewhere, run interrupted, session cleared) — then
+            # this really is a no-op expiry.
+            if not _clarify_entry_pending(self.clarify_id):
+                await self._edit_expired_embed(
+                    "⏱ Prompt expired — no action taken",
+                    discord.Color.greyple(),
+                )
+                return
+
+            # Buttons are dead but the clarify is still live agent-side, so
+            # flip it into text-capture mode: a typed reply now resolves the
+            # prompt instead of being rejected as "arbitrary prose for a
+            # multi-choice clarify" and silently queued behind the very turn
+            # it was meant to unblock.
+            grace = _read_clarify_text_grace()
+            flipped = False
+            try:
+                from tools.clarify_gateway import mark_awaiting_text
+                flipped = bool(mark_awaiting_text(self.clarify_id))
+            except Exception as exc:
+                logger.warning(
+                    "Discord clarify mark_awaiting_text on timeout failed (id=%s): %s",
+                    self.clarify_id, exc,
+                )
+
+            if flipped and grace > 0:
+                await self._edit_expired_embed(
+                    "⏱ Buttons expired — reply with a message to answer",
+                    discord.Color.orange(),
+                )
+            else:
+                await self._edit_expired_embed(
+                    "⏱ Prompt expired — no action taken",
+                    discord.Color.greyple(),
+                )
+
+            # Never leave the agent parked behind an unanswerable prompt.
+            task = asyncio.create_task(self._resolve_after_grace(grace))
+            _CLARIFY_EXPIRY_TASKS.add(task)
+            task.add_done_callback(_CLARIFY_EXPIRY_TASKS.discard)
 if DISCORD_AVAILABLE:
     _define_discord_view_classes()
 

tests/gateway/test_discord_clarify_buttons.py

@@ -294,3 +294,172 @@ async def test_unwrap_does_not_pick_value_or_name_alone(self):
         for label in choice_labels:
             assert "only_name_here" not in label, f"name leaked: {label!r}"
             assert "only_value_here" not in label, f"value leaked: {label!r}"
+
+
+# ===========================================================================
+# ClarifyChoiceView.on_timeout — expiry must release the agent
+# ===========================================================================
+
+def _make_view_message():
+    """Mock the prompt message the view repaints on expiry."""
+    embed = MagicMock()
+    embed.color = None
+    embed.set_footer = MagicMock()
+    return SimpleNamespace(embeds=[embed], edit=AsyncMock())
+
+
+def _expiry_tasks():
+    """Live expiry tasks. getattr keeps the behavioural assertions below the
+    real failure point when the release logic is missing entirely."""
+    from plugins.platforms.discord import adapter as adapter_mod
+    return list(getattr(adapter_mod, "_CLARIFY_EXPIRY_TASKS", ()))
+
+
+async def _drain_expiry_tasks():
+    """Await whatever ``on_timeout`` scheduled, so assertions see the result."""
+    tasks = _expiry_tasks()
+    for task in tasks:
+        await task
+    return tasks
+
+
+def _cancel_expiry_tasks():
+    for task in _expiry_tasks():
+        task.cancel()
+
+
+class TestClarifyChoiceViewTimeout:
+    """An expired view must never leave the agent parked on a dead prompt.
+
+    The view timeout only kills the buttons; the agent thread stays inside
+    ``clarify_gateway.wait_for_response`` until ``agent.clarify_timeout``
+    fires (an hour by default, never when set to 0). Until then the prompt
+    is unanswerable: the buttons are disabled and typed prose is rejected
+    by the multi-choice coercion, so every follow-up message queues behind
+    the very turn it was meant to unblock.
+    """
+
+    def setup_method(self):
+        _clear_clarify_state()
+        _cancel_expiry_tasks()
+
+    def teardown_method(self):
+        _cancel_expiry_tasks()
+
+    @pytest.mark.asyncio
+    async def test_timeout_flips_live_entry_to_awaiting_text(self):
+        from tools import clarify_gateway as cm
+        cm.register("cidT1", "sk-T1", "Pick", ["x", "y"])
+
+        view = ClarifyChoiceView(
+            choices=["x", "y"], clarify_id="cidT1", allowed_user_ids={"42"},
+        )
+        view._message = _make_view_message()
+
+        await view.on_timeout()
+
+        # Entry survives the button expiry and now accepts free text.
+        with cm._lock:
+            entry = cm._entries.get("cidT1")
+        assert entry is not None
+        assert entry.awaiting_text is True
+        assert not entry.event.is_set()
+        assert all(b.disabled for b in view.children)
+        footer = view._message.embeds[0].set_footer.call_args.kwargs["text"]
+        assert "reply with a message" in footer.lower()
+
+    @pytest.mark.asyncio
+    async def test_typed_prose_answers_the_prompt_after_timeout(self):
+        """Regression: prose was rejected while the buttons were dead."""
+        from tools import clarify_gateway as cm
+        cm.register("cidT2", "sk-T2", "What first?", ["Fix the bug", "Do the docs"])
+
+        # Live multi-choice prompt: prose is (correctly) not an answer.
+        assert cm.resolve_text_response_for_session("sk-T2", "cancel that") is False
+
+        view = ClarifyChoiceView(
+            choices=["Fix the bug", "Do the docs"],
+            clarify_id="cidT2",
+            allowed_user_ids={"42"},
+        )
+        view._message = _make_view_message()
+        await view.on_timeout()
+
+        # Buttons are gone, so the same prose must now resolve the clarify.
+        assert cm.resolve_text_response_for_session("sk-T2", "cancel that") is True
+        with cm._lock:
+            entry = cm._entries.get("cidT2")
+        assert entry.response == "cancel that"
+        assert entry.event.is_set()
+
+    @pytest.mark.asyncio
+    async def test_timeout_releases_agent_when_grace_disabled(self, monkeypatch):
+        from plugins.platforms.discord import adapter as adapter_mod
+        from tools import clarify_gateway as cm
+        monkeypatch.setattr(adapter_mod, "_read_clarify_text_grace", lambda: 0)
+        cm.register("cidT3", "sk-T3", "Pick", ["x", "y"])
+
+        view = ClarifyChoiceView(
+            choices=["x", "y"], clarify_id="cidT3", allowed_user_ids={"42"},
+        )
+        view._message = _make_view_message()
+
+        await view.on_timeout()
+        await _drain_expiry_tasks()
+
+        # Empty response = "user did not answer"; the waiter unblocks.
+        with cm._lock:
+            entry = cm._entries.get("cidT3")
+        assert entry is not None
+        assert entry.response == ""
+        assert entry.event.is_set()
+
+    @pytest.mark.asyncio
+    async def test_grace_task_leaves_answered_prompt_alone(self, monkeypatch):
+        """A reply during the grace window wins; the task must not overwrite it."""
+        from plugins.platforms.discord import adapter as adapter_mod
+        from tools import clarify_gateway as cm
+        monkeypatch.setattr(adapter_mod, "_read_clarify_text_grace", lambda: 0)
+        cm.register("cidT4", "sk-T4", "Pick", ["x", "y"])
+
+        view = ClarifyChoiceView(
+            choices=["x", "y"], clarify_id="cidT4", allowed_user_ids={"42"},
+        )
+        view._message = _make_view_message()
+        await view.on_timeout()
+
+        # Simulate wait_for_response returning: the waiter pops its entry.
+        with cm._lock:
+            entry = cm._entries.pop("cidT4")
+            cm._session_index.pop("sk-T4", None)
+        entry.response = "y"
+
+        await _drain_expiry_tasks()
+        assert entry.response == "y"
+
+    @pytest.mark.asyncio
+    async def test_timeout_without_entry_is_a_plain_expiry(self):
+        view = ClarifyChoiceView(
+            choices=["x"], clarify_id="cidGoneT", allowed_user_ids={"42"},
+        )
+        view._message = _make_view_message()
+
+        await view.on_timeout()
+
+        footer = view._message.embeds[0].set_footer.call_args.kwargs["text"]
+        assert "no action taken" in footer.lower()
+        assert not _expiry_tasks()
+
+    @pytest.mark.asyncio
+    async def test_timeout_survives_missing_message_reference(self):
+        from tools import clarify_gateway as cm
+        cm.register("cidT5", "sk-T5", "Pick", ["x"])
+
+        view = ClarifyChoiceView(
+            choices=["x"], clarify_id="cidT5", allowed_user_ids={"42"},
+        )
+        # No view._message (send_clarify never stored one).
+        await view.on_timeout()
+
+        with cm._lock:
+            assert cm._entries.get("cidT5") is not None