mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix: contain Mattermost bot peer loops
(cherry picked from commit b21f2e9fdc45335c55d200c2812f895d81ec28fd, #137037; conflicts resolved onto main: ALLOW_BOTS moved to the Mattermost gateway_env AUTHZ entry, status suppression moved to gateway/run_turn_progress.py)
This commit is contained in:
1 parent
d53973afa9
commit
cb52533765
7 files changed
+97
-1
No files matched your search
@@ -922,6 +922,9 @@ class GatewayBusySessionMixin:
|
||||
return message
|
||||
|
||||
async def _send_busy_ack_reply(self, event: MessageEvent, adapter, message: str) -> None:
|
||||
if getattr(event.source, "is_bot", False):
|
||||
logger.debug("Busy ack suppressed for bot-authored message in %s", event.source.chat_id)
|
||||
return
|
||||
try:
|
||||
await self._send_busy_reply(event, adapter, message)
|
||||
except Exception as e:
|
||||
|
||||
@@ -987,6 +987,11 @@ class GatewayTurnProgressMixin:
|
||||
ctx = self._ctx
|
||||
if ctx.mute_notification_reply or not self._status_live():
|
||||
return
|
||||
# Lifecycle diagnostics (including provider fallback notices) are UI affordances for a
|
||||
# human caller. Sending them to a known bot peer can create a new inbound turn and a loop.
|
||||
if getattr(ctx.source, "is_bot", False):
|
||||
logger.debug("status_callback suppressed for bot-authored message: %s", event_type)
|
||||
return
|
||||
prepared = _prepare_gateway_status_message(ctx.source.platform, event_type, message)
|
||||
if prepared is None:
|
||||
logger.debug(
|
||||
|
||||
@@ -124,6 +124,17 @@ class MattermostAdapter(BasePlatformAdapter):
|
||||
self._last_post_error: str = ""
|
||||
self._dedup = MessageDeduplicator()
|
||||
|
||||
def _is_configured_bot_user(self, user_id: str) -> bool:
|
||||
"""Whether an inbound Mattermost sender is an explicitly configured peer bot.
|
||||
|
||||
Mattermost's posted WebSocket event contains only a sender ID, not a bot flag. Keep the
|
||||
classification opt-in: an unknown account stays a human rather than silently changing its
|
||||
authorization or delivery behavior.
|
||||
"""
|
||||
extra = getattr(self.config, "extra", {}) or {}
|
||||
configured = extra.get("bot_users", _get_scoped_secret("MATTERMOST_BOT_USERS", ""))
|
||||
return str(user_id) in _channel_id_set(configured)
|
||||
|
||||
# --- HTTP helpers ---
|
||||
|
||||
def _headers(self) -> dict[str, str]:
|
||||
@@ -583,7 +594,7 @@ class MattermostAdapter(BasePlatformAdapter):
|
||||
source = self.build_source(
|
||||
chat_id=channel_id, chat_type=_CHANNEL_TYPE_MAP.get(data.get("channel_type", "O"), "channel"),
|
||||
user_id=sender_id, user_name=data.get("sender_name", "").lstrip("@") or sender_id,
|
||||
thread_id=thread_id, message_id=post_id)
|
||||
thread_id=thread_id, message_id=post_id, is_bot=self._is_configured_bot_user(sender_id))
|
||||
from gateway.platforms.base import resolve_channel_prompt
|
||||
await self.handle_message(MessageEvent(
|
||||
text=message_text, message_type=msg_type, source=source, raw_message=post, message_id=post_id,
|
||||
|
||||
@@ -18,5 +18,6 @@ ENV_OVERRIDES = (
|
||||
AUTHZ = {
|
||||
"allowed_users_env": "MATTERMOST_ALLOWED_USERS",
|
||||
"allow_all_env": "MATTERMOST_ALLOW_ALL_USERS",
|
||||
"allow_bots_env": "MATTERMOST_ALLOW_BOTS",
|
||||
"allowlist_suppresses_pairing": True,
|
||||
}
|
||||
@@ -97,6 +97,20 @@ def _make_adapter(platform_val="telegram"):
|
||||
class TestBusySessionAck:
|
||||
"""User sends a message while agent is running — should get acknowledgment."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_busy_ack_is_not_sent_to_a_bot_peer(self):
|
||||
"""Regression for #137027: runtime acknowledgements must not restart bot loops."""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
runner, _sentinel = _make_runner()
|
||||
event = _make_event()
|
||||
event.source.is_bot = True
|
||||
adapter = _make_adapter()
|
||||
|
||||
await GatewayRunner._send_busy_ack_reply(runner, event, adapter, "⚡ Interrupting current task")
|
||||
|
||||
adapter._send_with_retry.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_telegram_grace_followups_respect_queue_fifo(self, monkeypatch):
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import json
|
||||
import os
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
import pytest
|
||||
from unittest.mock import MagicMock, patch, AsyncMock
|
||||
|
||||
@@ -299,6 +300,23 @@ class TestMattermostWebSocketParsing:
|
||||
assert msg_event.text == "Hello from Matrix!"
|
||||
assert msg_event.message_id == "post_abc"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_known_peer_bot_is_marked_on_the_inbound_source(self):
|
||||
"""Regression for #137027: configured peer bots must enter the loop guard."""
|
||||
self.adapter.config.extra["bot_users"] = ["peer_bot"]
|
||||
self.adapter._download_attachments = AsyncMock(return_value=([], []))
|
||||
post_data = {
|
||||
"id": "post_from_peer", "user_id": "peer_bot", "channel_id": "dm_456", "message": "ping",
|
||||
}
|
||||
event = {
|
||||
"event": "posted",
|
||||
"data": {"post": json.dumps(post_data), "channel_type": "D", "sender_name": "@peer"},
|
||||
}
|
||||
|
||||
await self.adapter._handle_ws_event(event)
|
||||
|
||||
assert self.adapter.handle_message.await_args.args[0].source.is_bot is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ignore_system_posts(self):
|
||||
@@ -709,3 +727,43 @@ class TestMultiplexProfileScope:
|
||||
# os.environ.
|
||||
assert "MATTERMOST_REQUIRE_MENTION" not in os.environ
|
||||
|
||||
|
||||
def test_bot_turn_does_not_publish_runtime_status():
|
||||
"""#137027: a provider-fallback diagnostic must not become a peer-bot message."""
|
||||
from gateway.run_turn_runner import TurnRunner
|
||||
|
||||
turn_runner = object.__new__(TurnRunner)
|
||||
turn_runner._schedule = MagicMock()
|
||||
turn_runner._ctx = SimpleNamespace(
|
||||
mute_notification_reply=False,
|
||||
source=SimpleNamespace(platform=Platform.MATTERMOST, is_bot=True),
|
||||
_status_adapter=object(),
|
||||
_run_still_current=lambda: True,
|
||||
user_config={},
|
||||
_status_chat_id="dm_456",
|
||||
_status_thread_metadata=None,
|
||||
_cleanup_progress=False,
|
||||
)
|
||||
|
||||
turn_runner._status_callback_sync("warn", "⚠️ Model fallback: unavailable")
|
||||
|
||||
turn_runner._schedule.assert_not_called()
|
||||
|
||||
|
||||
def test_configured_mattermost_bot_is_authorized(monkeypatch):
|
||||
"""#137027: peer bots use Mattermost's registered bot-admission policy."""
|
||||
from gateway.run import GatewayRunner
|
||||
from gateway.session import SessionSource
|
||||
|
||||
for name in ("MATTERMOST_ALLOW_BOTS", "MATTERMOST_ALLOWED_USERS", "GATEWAY_ALLOW_ALL_USERS", "GATEWAY_ALLOWED_USERS"):
|
||||
monkeypatch.delenv(name, raising=False)
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner.pairing_store = SimpleNamespace(is_approved=lambda *_a, **_kw: False)
|
||||
source = SessionSource(
|
||||
platform=Platform.MATTERMOST, chat_id="dm_456", chat_type="dm", user_id="peer_bot",
|
||||
user_name="Peer Bot", is_bot=True,
|
||||
)
|
||||
|
||||
assert runner._is_user_authorized(source) is False
|
||||
monkeypatch.setenv("MATTERMOST_ALLOW_BOTS", "mentions")
|
||||
assert runner._is_user_authorized(source) is True
|
||||
@@ -159,6 +159,10 @@ MATTERMOST_ALLOWED_USERS=3uo8dkh1p7g1mfk49ear5fzs5c
|
||||
|
||||
# Optional: channels where bot responds without @mention (comma-separated channel IDs)
|
||||
# MATTERMOST_FREE_RESPONSE_CHANNELS=channel_id_1,channel_id_2
|
||||
|
||||
# Optional: peer bot user IDs. These senders use the bot loop guard; set
|
||||
# MATTERMOST_ALLOW_BOTS=all (or mentions) only when bot-to-bot messages are intentional.
|
||||
# MATTERMOST_BOT_USERS=peer_bot_user_id_1,peer_bot_user_id_2
|
||||
```
|
||||
|
||||
Optional behavior settings in `~/.hermes/config.yaml`:
|
||||
|
||||
Reference in new issue
Block a user