mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
feat(discord): channel history backfill for multi-user sessions
Adds optional channel-context backfill for Discord shared-channel sessions so the agent can see recent messages it missed between its own turns (typically when require_mention=true filters out most traffic). Previously the agent only saw the @mention message that triggered it, which led to disorienting replies in active multi-user channels where the conversation context was invisible. With backfill enabled, a configurable number of recent messages are fetched per-turn and prepended to the trigger message as a context block, kept separate from sender-prefix logic so attribution remains clean. This re-opens the work from #13063 (approved by @OutThisLife on 2026-04-20, closed when I closed the branch to address the simpolism:main head-branch issue plus an ordering bug I caught later in live use). Filing against the freshly-rewritten problem statement in #13054 so the design is grounded in the failure mode rather than the implementation shape. The implementation follows the **push-mode last-self-anchored** design from the two options laid out in #13054. See the issue for the trade-off discussion vs pull-mode (#13120 was an earlier closed PR using that shape). Treating this as a reference implementation — happy to rewrite as last-trigger anchoring or as a hybrid with #13120 if maintainers prefer. Changes: - gateway/platforms/discord.py: - new `_discord_history_backfill()` / `_discord_history_backfill_limit()` helpers (config.extra > env > default), mirroring the existing `_discord_require_mention()` shape - new `_fetch_channel_context()` that scans `channel.history()` backwards from the trigger to the bot's last message (or limit), formats as `[Recent channel messages] / [name] msg / ...`, respects DISCORD_ALLOW_BOTS, skips system messages - per-channel `_last_self_message_id` cache to narrow the fetch window on hot paths (avoids full history scan when the bot has spoken recently) - **IMPORTANT**: passes `oldest_first=False` explicitly to `channel.history()`. discord.py 2.x silently flips the default to True when `after=` is supplied, which would select the EARLIEST N messages after our last response instead of the LATEST N before the trigger. In high-traffic windows this would return stale tool traces and drop the actual final answer the user is asking about. See regression test below. Caught in live use during a Codex tool-trace burst on May 13 2026. - gateway/config.py: discord_history_backfill + discord_history_backfill_limit settings + yaml→env bridge - gateway/platforms/base.py: channel_context field on MessageEvent - gateway/run.py: prepend channel_context after sender-prefix so the [sender name] tag applies to the trigger message alone, not to the backfill - hermes_cli/config.py: defaults for new discord.history_backfill and discord.history_backfill_limit keys - cli-config.yaml.example: documented defaults - tests/gateway/test_discord_free_response.py: 7 new tests covering cold-start backfill, self-message stop boundary, other-bot filtering, cache hot-path narrowing, stale-cache fallback, shared-channel + per-user backfill paths, and the ordering regression test (`test_fetch_channel_context_cache_uses_latest_window_when_after_set`) - tests/gateway/test_config.py: yaml→env bridge tests - tests/gateway/test_session.py: prefix-order edge cases - website/docs/user-guide/messaging/discord.md: env vars + config keys + usage docs Tested on Ubuntu 24.04 — empirically validated in my own multi-bot Discord research server for the past three weeks. Fixes #13054 Supersedes #13063 (closed)
This commit is contained in:
snav
Teknium
parent
ccb5aae0d2
commit
e84fe483bc
10 changed files with 596 additions and 2 deletions
|
|
@ -409,6 +409,26 @@ class TestLoadGatewayConfig:
|
|||
"456": "Therapist mode",
|
||||
}
|
||||
|
||||
def test_bridges_discord_history_backfill_settings_from_config_yaml(self, tmp_path, monkeypatch):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"discord:\n"
|
||||
" history_backfill: true\n"
|
||||
" history_backfill_limit: 17\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.delenv("DISCORD_HISTORY_BACKFILL", raising=False)
|
||||
monkeypatch.delenv("DISCORD_HISTORY_BACKFILL_LIMIT", raising=False)
|
||||
|
||||
load_gateway_config()
|
||||
|
||||
assert os.getenv("DISCORD_HISTORY_BACKFILL") == "true"
|
||||
assert os.getenv("DISCORD_HISTORY_BACKFILL_LIMIT") == "17"
|
||||
|
||||
def test_bridges_telegram_channel_prompts_from_config_yaml(self, tmp_path, monkeypatch):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
|
|
|
|||
|
|
@ -62,6 +62,12 @@ class FakeTextChannel:
|
|||
self.guild = SimpleNamespace(name=guild_name)
|
||||
self.topic = None
|
||||
|
||||
def history(self, *, limit, before, after=None, oldest_first=None):
|
||||
async def _iter():
|
||||
return
|
||||
yield
|
||||
return _iter()
|
||||
|
||||
|
||||
class FakeForumChannel:
|
||||
def __init__(self, channel_id: int = 1, name: str = "support-forum", guild_name: str = "Hermes Server"):
|
||||
|
|
@ -99,6 +105,9 @@ def adapter(monkeypatch):
|
|||
"DISCORD_NO_THREAD_CHANNELS",
|
||||
"DISCORD_ALLOWED_CHANNELS",
|
||||
"DISCORD_IGNORED_CHANNELS",
|
||||
"DISCORD_HISTORY_BACKFILL",
|
||||
"DISCORD_HISTORY_BACKFILL_LIMIT",
|
||||
"DISCORD_ALLOW_BOTS",
|
||||
):
|
||||
monkeypatch.delenv(_var, raising=False)
|
||||
|
||||
|
|
@ -125,6 +134,48 @@ def make_message(*, channel, content: str, mentions=None, msg_type=None):
|
|||
)
|
||||
|
||||
|
||||
def make_history_message(
|
||||
*,
|
||||
author,
|
||||
content: str,
|
||||
msg_id: int,
|
||||
msg_type=None,
|
||||
attachments=None,
|
||||
):
|
||||
return SimpleNamespace(
|
||||
id=msg_id,
|
||||
author=author,
|
||||
content=content,
|
||||
attachments=list(attachments or []),
|
||||
type=msg_type if msg_type is not None else discord_platform.discord.MessageType.default,
|
||||
)
|
||||
|
||||
|
||||
class FakeHistoryChannel(FakeTextChannel):
|
||||
def __init__(self, history_messages, **kwargs):
|
||||
super().__init__(**kwargs)
|
||||
self._history_messages = list(history_messages)
|
||||
|
||||
def history(self, *, limit, before, after=None, oldest_first=None):
|
||||
before_id = int(getattr(before, "id", before))
|
||||
after_id = int(getattr(after, "id", after)) if after is not None else None
|
||||
if oldest_first is None:
|
||||
oldest_first = after is not None
|
||||
|
||||
messages = [
|
||||
message for message in self._history_messages
|
||||
if int(message.id) < before_id
|
||||
and (after_id is None or int(message.id) > after_id)
|
||||
]
|
||||
messages.sort(key=lambda message: int(message.id), reverse=not oldest_first)
|
||||
|
||||
async def _iter():
|
||||
for message in messages[:limit]:
|
||||
yield message
|
||||
|
||||
return _iter()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discord_defaults_to_require_mention(adapter, monkeypatch):
|
||||
"""Default behavior: require @mention in server channels."""
|
||||
|
|
@ -578,3 +629,217 @@ async def test_discord_thread_require_mention_via_config_extra(adapter, monkeypa
|
|||
await adapter._handle_message(message)
|
||||
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_stops_at_self_message_and_reverses_to_chronological_order(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "all")
|
||||
adapter.config.extra["history_backfill_limit"] = 10
|
||||
|
||||
other_bot = SimpleNamespace(id=55, display_name="Gemini", name="Gemini", bot=True)
|
||||
human = SimpleNamespace(id=56, display_name="Alice", name="Alice", bot=False)
|
||||
old_human = SimpleNamespace(id=57, display_name="Bob", name="Bob", bot=False)
|
||||
|
||||
channel = FakeHistoryChannel(
|
||||
[
|
||||
make_history_message(author=human, content="latest human note", msg_id=4),
|
||||
make_history_message(author=other_bot, content="latest bot note", msg_id=3),
|
||||
make_history_message(author=adapter._client.user, content="our prior response", msg_id=2),
|
||||
make_history_message(author=old_human, content="older than boundary", msg_id=1),
|
||||
],
|
||||
channel_id=123,
|
||||
)
|
||||
|
||||
result = await adapter._fetch_channel_context(channel, before=make_message(channel=channel, content="trigger"))
|
||||
|
||||
assert result == (
|
||||
"[Recent channel messages]\n"
|
||||
"[Gemini [bot]] latest bot note\n"
|
||||
"[Alice] latest human note"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_skips_other_bots_when_allow_bots_none(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "none")
|
||||
adapter.config.extra["history_backfill_limit"] = 10
|
||||
|
||||
other_bot = SimpleNamespace(id=55, display_name="Gemini", name="Gemini", bot=True)
|
||||
human = SimpleNamespace(id=56, display_name="Alice", name="Alice", bot=False)
|
||||
|
||||
channel = FakeHistoryChannel(
|
||||
[
|
||||
make_history_message(author=human, content="human note", msg_id=3),
|
||||
make_history_message(author=other_bot, content="bot note", msg_id=2),
|
||||
],
|
||||
channel_id=123,
|
||||
)
|
||||
|
||||
result = await adapter._fetch_channel_context(channel, before=make_message(channel=channel, content="trigger"))
|
||||
|
||||
assert result == "[Recent channel messages]\n[Alice] human note"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_uses_cache_to_narrow_window(adapter, monkeypatch):
|
||||
"""When _last_self_message_id is cached, the fetch passes after= to skip old messages."""
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "all")
|
||||
adapter.config.extra["history_backfill_limit"] = 50
|
||||
|
||||
human = SimpleNamespace(id=56, display_name="Alice", name="Alice", bot=False)
|
||||
|
||||
# Record the after= arg passed to history()
|
||||
recorded_after = {}
|
||||
|
||||
class CacheTrackingChannel(FakeHistoryChannel):
|
||||
def history(self, *, limit, before, after=None, oldest_first=None):
|
||||
recorded_after["value"] = after
|
||||
return super().history(
|
||||
limit=limit,
|
||||
before=before,
|
||||
after=after,
|
||||
oldest_first=oldest_first,
|
||||
)
|
||||
|
||||
channel = CacheTrackingChannel(
|
||||
[make_history_message(author=human, content="hello", msg_id=200)],
|
||||
channel_id=777,
|
||||
)
|
||||
|
||||
# Seed the cache — bot's last message in this channel was ID 100
|
||||
adapter._last_self_message_id["777"] = "100"
|
||||
|
||||
trigger = make_message(channel=channel, content="trigger")
|
||||
trigger.id = 300 # trigger is newer than cache
|
||||
|
||||
result = await adapter._fetch_channel_context(channel, before=trigger)
|
||||
|
||||
assert result == "[Recent channel messages]\n[Alice] hello"
|
||||
# Verify cache was used: after= should be set (not None)
|
||||
assert recorded_after["value"] is not None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_cache_uses_latest_window_when_after_set(adapter, monkeypatch):
|
||||
"""Regression: discord.py defaults oldest_first=True when after= is provided.
|
||||
|
||||
The hot cache path passes both after= and before=. We still want the latest
|
||||
messages before the trigger, not the earliest messages after our prior
|
||||
response, otherwise tool traces can crowd out the final answer.
|
||||
"""
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "all")
|
||||
adapter.config.extra["history_backfill_limit"] = 3
|
||||
|
||||
codex = SimpleNamespace(id=56, display_name="Codex", name="Codex", bot=True)
|
||||
human = SimpleNamespace(id=57, display_name="Alice", name="Alice", bot=False)
|
||||
|
||||
channel = FakeHistoryChannel(
|
||||
[
|
||||
make_history_message(author=codex, content="old tool trace 1", msg_id=101),
|
||||
make_history_message(author=codex, content="old tool trace 2", msg_id=102),
|
||||
make_history_message(author=codex, content="old tool trace 3", msg_id=103),
|
||||
make_history_message(author=codex, content="final analysis", msg_id=104),
|
||||
make_history_message(author=human, content="latest follow-up", msg_id=105),
|
||||
],
|
||||
channel_id=777,
|
||||
)
|
||||
adapter._last_self_message_id["777"] = "100"
|
||||
|
||||
trigger = make_message(channel=channel, content="trigger")
|
||||
trigger.id = 200
|
||||
|
||||
result = await adapter._fetch_channel_context(channel, before=trigger)
|
||||
|
||||
assert "[Codex [bot]] final analysis" in result
|
||||
assert "[Alice] latest follow-up" in result
|
||||
assert "old tool trace 1" not in result
|
||||
assert "old tool trace 2" not in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fetch_channel_context_ignores_stale_cache(adapter, monkeypatch):
|
||||
"""If cached ID is >= trigger ID (stale/future), fall back to cold-start scan."""
|
||||
monkeypatch.setenv("DISCORD_ALLOW_BOTS", "all")
|
||||
adapter.config.extra["history_backfill_limit"] = 50
|
||||
|
||||
human = SimpleNamespace(id=56, display_name="Alice", name="Alice", bot=False)
|
||||
|
||||
recorded_after = {}
|
||||
|
||||
class CacheTrackingChannel(FakeHistoryChannel):
|
||||
def history(self, *, limit, before, after=None, oldest_first=None):
|
||||
recorded_after["value"] = after
|
||||
return super().history(
|
||||
limit=limit,
|
||||
before=before,
|
||||
after=after,
|
||||
oldest_first=oldest_first,
|
||||
)
|
||||
|
||||
channel = CacheTrackingChannel(
|
||||
[make_history_message(author=human, content="hello", msg_id=50)],
|
||||
channel_id=777,
|
||||
)
|
||||
|
||||
# Cache has a NEWER ID than the trigger — stale/invalid
|
||||
adapter._last_self_message_id["777"] = "500"
|
||||
|
||||
trigger = make_message(channel=channel, content="trigger")
|
||||
trigger.id = 300
|
||||
|
||||
result = await adapter._fetch_channel_context(channel, before=trigger)
|
||||
|
||||
assert result == "[Recent channel messages]\n[Alice] hello"
|
||||
# Cache should have been ignored — after= should be None
|
||||
assert recorded_after["value"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discord_shared_channel_backfill_prepends_context(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "true")
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
monkeypatch.setenv("DISCORD_AUTO_THREAD", "false")
|
||||
adapter.config.extra["group_sessions_per_user"] = False
|
||||
adapter.config.extra["history_backfill"] = True
|
||||
adapter._fetch_channel_context = AsyncMock(return_value="[Recent channel messages]\n[Alice] context")
|
||||
|
||||
bot_user = adapter._client.user
|
||||
message = make_message(
|
||||
channel=FakeTextChannel(channel_id=321),
|
||||
content=f"<@{bot_user.id}> hello with mention",
|
||||
mentions=[bot_user],
|
||||
)
|
||||
|
||||
await adapter._handle_message(message)
|
||||
|
||||
adapter._fetch_channel_context.assert_awaited_once()
|
||||
event = adapter.handle_message.await_args.args[0]
|
||||
assert event.text == "hello with mention"
|
||||
assert event.channel_context == "[Recent channel messages]\n[Alice] context"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discord_per_user_channel_does_not_backfill(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "true")
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
monkeypatch.setenv("DISCORD_AUTO_THREAD", "false")
|
||||
adapter.config.extra["group_sessions_per_user"] = True
|
||||
adapter.config.extra["history_backfill"] = True
|
||||
adapter._fetch_channel_context = AsyncMock(return_value="[Recent channel messages]\n[Alice] context")
|
||||
|
||||
bot_user = adapter._client.user
|
||||
message = make_message(
|
||||
channel=FakeTextChannel(channel_id=321),
|
||||
content=f"<@{bot_user.id}> hello with mention",
|
||||
mentions=[bot_user],
|
||||
)
|
||||
|
||||
await adapter._handle_message(message)
|
||||
|
||||
adapter._fetch_channel_context.assert_not_awaited()
|
||||
event = adapter.handle_message.await_args.args[0]
|
||||
assert event.text == "hello with mention"
|
||||
assert event.channel_context is None
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import pytest
|
|||
from pathlib import Path
|
||||
from unittest.mock import patch, MagicMock
|
||||
from gateway.config import Platform, HomeChannel, GatewayConfig, PlatformConfig
|
||||
from gateway.platforms.base import MessageEvent
|
||||
from gateway.session import (
|
||||
SessionSource,
|
||||
SessionStore,
|
||||
|
|
@ -430,6 +431,76 @@ class TestBuildSessionContextPrompt:
|
|||
assert "Multi-user thread" not in prompt
|
||||
|
||||
|
||||
class TestSenderPrefixWithBackfill:
|
||||
"""Regression: sender prefix must not wrap the backfill context block.
|
||||
|
||||
Tests exercise the real GatewayRunner._prepare_inbound_message_text()
|
||||
method to ensure the [sender_name] prefix applies only to the trigger
|
||||
message, not the channel_context backfill block.
|
||||
"""
|
||||
|
||||
@pytest.fixture()
|
||||
def runner(self):
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
r = GatewayRunner.__new__(GatewayRunner)
|
||||
r.config = GatewayConfig(group_sessions_per_user=False)
|
||||
r.adapters = {}
|
||||
r._model = "test-model"
|
||||
r._base_url = ""
|
||||
r._has_setup_skill = lambda: False
|
||||
return r
|
||||
|
||||
@pytest.fixture()
|
||||
def source(self):
|
||||
return SessionSource(
|
||||
platform=Platform.DISCORD,
|
||||
chat_id="c1",
|
||||
chat_type="group",
|
||||
user_name="Alice",
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plain_message_gets_prefix(self, runner, source):
|
||||
"""Normal message without backfill gets [sender] prefix."""
|
||||
event = MessageEvent(text="hello world", source=source)
|
||||
result = await runner._prepare_inbound_message_text(
|
||||
event=event, source=source, history=[],
|
||||
)
|
||||
assert result == "[Alice] hello world"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_backfill_prefix_only_on_trigger(self, runner, source):
|
||||
"""Backfill context must NOT get the sender prefix."""
|
||||
event = MessageEvent(
|
||||
text="hello world",
|
||||
source=source,
|
||||
channel_context="[Recent channel messages]\n[Bob] some context",
|
||||
)
|
||||
result = await runner._prepare_inbound_message_text(
|
||||
event=event, source=source, history=[],
|
||||
)
|
||||
assert result.startswith("[Recent channel messages]")
|
||||
assert "[Alice] [Recent channel messages]" not in result
|
||||
assert "[New message]\n[Alice] hello world" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_backfill_preserves_context_block(self, runner, source):
|
||||
"""The backfill block should pass through unchanged — no double-prefixing."""
|
||||
context = "[Recent channel messages]\n[Bob] first\n[Charlie [bot]] second"
|
||||
event = MessageEvent(
|
||||
text="hey everyone", source=source, channel_context=context,
|
||||
)
|
||||
result = await runner._prepare_inbound_message_text(
|
||||
event=event, source=source, history=[],
|
||||
)
|
||||
assert result.startswith(context)
|
||||
assert "[Alice] hey everyone" in result
|
||||
assert "[Alice] [Bob]" not in result
|
||||
assert "[Alice] [Charlie" not in result
|
||||
assert "[Alice] [Recent" not in result
|
||||
|
||||
|
||||
class TestSessionStoreRewriteTranscript:
|
||||
"""Regression: /retry and /undo must persist truncated history to disk."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue