diff --git a/gateway/profile_routing.py b/gateway/profile_routing.py index 836b50639c70..205f36d0630c 100644 --- a/gateway/profile_routing.py +++ b/gateway/profile_routing.py @@ -4,15 +4,16 @@ Allows a single Hermes instance to route specific Discord guilds/channels/thread to different profiles — each with their own model, tools, memory, and persona. Matching priority (most specific first): - 1. platform + chat_id + thread_id (exact thread) — specificity 8 - 2. platform + chat_id (channel route) — specificity 4 + 1. platform + chat_id + thread_id (exact thread) — specificity 14 + 2. platform + chat_id (channel route) — specificity 6 3. platform + guild_id (guild/server route) — specificity 2 4. No match → default profile -Hierarchical matching: -For Discord forum channels, checks the full parent chain: -- Forum channel → Forum post → Comment -- Matches if any level of the hierarchy matches a configured route +Parent-chain matching: +For Discord threads and forum posts, ``parent_chat_id`` carries the +direct parent (the channel for a thread, the forum channel for a post). +Routes keyed on a channel match both direct messages and messages in +any thread/post whose parent is that channel. Configuration (config.yaml): @@ -38,34 +39,14 @@ Configuration (config.yaml): from __future__ import annotations -from collections import OrderedDict from dataclasses import dataclass -from typing import Any, Dict, List, Optional, Set +from typing import Any, Dict, List, Optional import logging logger = logging.getLogger(__name__) -# Bounded LRU cache for forum post to channel mappings. -# OrderedDict evicts least-recently-used entries when full. -_MAX_FORUM_CACHE = 10000 -_forum_post_cache: OrderedDict[str, str] = OrderedDict() # post_id -> channel_id - -def register_forum_post(post_id: str, channel_id: str) -> None: - """Register a forum post's parent channel for hierarchical matching.""" - _forum_post_cache[post_id] = channel_id - _forum_post_cache.move_to_end(post_id) - while len(_forum_post_cache) > _MAX_FORUM_CACHE: - _forum_post_cache.popitem(last=False) - logger.debug("Registered forum post %s -> channel %s", post_id, channel_id) - - -def resolve_forum_channel(post_id: str) -> Optional[str]: - """Get the parent channel ID for a forum post, if cached.""" - return _forum_post_cache.get(post_id) - - @dataclass(frozen=True) class ProfileRoute: """A single routing rule that maps a platform scope to a profile.""" @@ -103,8 +84,6 @@ class ProfileRoute: Supports hierarchical matching for Discord forums: - Direct channel match: chat_id == route.chat_id - Thread in channel: parent_chat_id == route.chat_id - - Forum post: parent_chat_id is the forum post, check if post belongs to route's channel - - Comment on forum post: parent_chat_id is the forum post, check hierarchy """ if not self.enabled: return False @@ -121,12 +100,6 @@ class ProfileRoute: # Parent match (thread or direct child) if self.chat_id == parent_chat_id: return True - # Forum post hierarchy: check if parent_chat_id is a forum post - # that belongs to this channel - if parent_chat_id: - parent_channel = resolve_forum_channel(parent_chat_id) - if parent_channel and self.chat_id == parent_channel: - return True # If chat_id was specified but didn't match any level, fail if self.chat_id: diff --git a/gateway/run.py b/gateway/run.py index 3094ebf59cbd..83c5bf9a31ee 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -17342,16 +17342,50 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew fallback for sources that bypass ``build_source``. 3. The active profile (the multiplexer's own home). """ - from hermes_cli.profiles import get_active_profile_name, get_profile_dir + from hermes_cli.profiles import ( + get_active_profile_name, + get_profile_dir, + profile_exists, + ) + from hermes_constants import get_hermes_home + + # Track whether a profile was explicitly requested (vs. falling back to default) + explicit_profile = None try: name = (source.profile or "").strip() + if name: + explicit_profile = name # User explicitly set this profile if not name: name = self._profile_name_for_source(source) + if name: + explicit_profile = name # Routing explicitly set this profile if not name: name = get_active_profile_name() or "default" - return get_profile_dir(name) + + profile_dir = get_profile_dir(name) + # Warn if an explicit profile doesn't exist on disk + if explicit_profile and not profile_exists(name): + logger.warning( + "Profile %r does not exist for source %s/%s (guild_id=%s), " + "falling back to global HERMES_HOME", + explicit_profile, + source.platform.value, + source.chat_id, + getattr(source, "guild_id", None), + ) + return get_hermes_home() + return profile_dir except Exception: - from hermes_constants import get_hermes_home + # Catch normalization errors, path errors, etc. + logger.warning( + "Failed to resolve profile directory for source %s/%s (guild_id=%s), " + "falling back to global HERMES_HOME: %s", + source.platform.value, + source.chat_id, + getattr(source, "guild_id", None), + explicit_profile or "(no profile)", + exc_info=True, + ) return get_hermes_home() async def _run_agent_inner( diff --git a/tests/gateway/test_profile_resolution.py b/tests/gateway/test_profile_resolution.py new file mode 100644 index 000000000000..da0ddf32bfb3 --- /dev/null +++ b/tests/gateway/test_profile_resolution.py @@ -0,0 +1,257 @@ +"""Tests for GatewayRunner._resolve_profile_home_for_source — profile resolution logic.""" + +import logging +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from gateway.session import SessionSource +from gateway.run import GatewayRunner + + +@pytest.fixture +def mock_runner(): + """Create a minimal mock GatewayRunner with the methods we need.""" + runner = MagicMock(spec=GatewayRunner) + runner.config = MagicMock(profile_routes=[]) + # Bind the actual methods to the mock + runner._profile_name_for_source = GatewayRunner._profile_name_for_source.__get__(runner) + runner._resolve_profile_home_for_source = GatewayRunner._resolve_profile_home_for_source.__get__(runner) + return runner + + +@pytest.fixture +def discord_source(): + """Create a basic Discord SessionSource for testing.""" + return SessionSource( + platform=MagicMock(value="discord"), + chat_id="123456", + guild_id="789", + thread_id=None, + parent_chat_id=None, + ) + + +class TestResolutionOrder: + """Tests that profile resolution follows the correct priority order.""" + + def test_source_profile_wins_over_routing(self, mock_runner, discord_source): + """source.profile should be used even if routing would match.""" + discord_source.profile = "from-source" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + with patch("hermes_cli.profiles.profile_exists", return_value=True): + mock_get_dir.return_value = Path("/hermes/profiles/from-source") + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/from-source") + mock_get_dir.assert_called_once_with("from-source") + + def test_routing_wins_over_active_profile(self, mock_runner, discord_source): + """When source.profile is empty, routing should win over active profile.""" + discord_source.profile = None + + # Mock routing to return a profile + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + with patch("hermes_cli.profiles.profile_exists", return_value=True): + mock_get_dir.return_value = Path("/hermes/profiles/routed") + + # Manually set routing to return a profile + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/routed") + mock_get_dir.assert_called_once_with("routed") + + def test_active_profile_fallback(self, mock_runner, discord_source): + """When source.profile and routing both return None, active profile is used.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/active") + + # No routing match + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/active") + mock_get_dir.assert_called_once_with("active") + + def test_default_fallback_when_no_active(self, mock_runner, discord_source): + """When even active profile is None, 'default' is used.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value=None): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes") + + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes") + mock_get_dir.assert_called_once_with("default") + + +class TestMissingProfileWarning: + """Tests for warning when a profile doesn't exist on disk.""" + + def test_nonexistent_profile_warning(self, mock_runner, discord_source, caplog): + """When source.profile points to a nonexistent profile, log a WARNING.""" + discord_source.profile = "nonexistent" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/nonexistent") + with patch("hermes_cli.profiles.profile_exists", return_value=False): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should fall back to global HERMES_HOME + assert result == Path("/hermes") + + # Should have logged a warning + assert len(caplog.records) == 1 + assert caplog.records[0].levelname == "WARNING" + assert "nonexistent" in caplog.records[0].message + assert "does not exist" in caplog.records[0].message + assert "discord" in caplog.records[0].message + assert "123456" in caplog.records[0].message + + def test_nonexistent_routing_profile_warning(self, mock_runner, discord_source, caplog): + """When routing returns a nonexistent profile, log a WARNING.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/routed") + with patch("hermes_cli.profiles.profile_exists", return_value=False): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + # Routing returns a profile that doesn't exist + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should fall back to global HERMES_HOME + assert result == Path("/hermes") + + # Should have logged a warning + assert len(caplog.records) == 1 + assert "routed" in caplog.records[0].message + + def test_empty_source_profile_no_warning(self, mock_runner, discord_source, caplog): + """When source.profile is empty, silent fallback to active profile (no warning).""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/active") + with patch("hermes_cli.profiles.profile_exists", return_value=True): + with caplog.at_level(logging.WARNING): + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should use active profile + assert result == Path("/hermes/profiles/active") + + # No warnings (active profile exists) + assert not any(r.levelname == "WARNING" for r in caplog.records) + + def test_existing_profile_no_warning(self, mock_runner, discord_source, caplog): + """When the profile exists, no warning should be logged.""" + discord_source.profile = "existing" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/existing") + with patch("hermes_cli.profiles.profile_exists", return_value=True): + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/existing") + + # No warnings + assert not any(r.levelname == "WARNING" for r in caplog.records) + + +class TestExceptionHandling: + """Tests for exception handling in profile resolution.""" + + def test_get_profile_dir_exception_logs_warning(self, mock_runner, discord_source, caplog): + """When get_profile_dir raises an exception, log a WARNING with context.""" + discord_source.profile = "bad-profile" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir", side_effect=ValueError("Invalid profile name")): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should fall back to global HERMES_HOME + assert result == Path("/hermes") + + # Should have logged a warning with exception info + assert len(caplog.records) == 1 + assert caplog.records[0].levelname == "WARNING" + assert "bad-profile" in caplog.records[0].message + assert "Failed to resolve profile directory" in caplog.records[0].message + + def test_exception_with_no_profile_name(self, mock_runner, discord_source, caplog): + """Exception when no profile was set should still log a warning.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value=None): + with patch("hermes_cli.profiles.get_profile_dir", side_effect=RuntimeError("Filesystem error")): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes") + + # Warning should mention "(no profile)" + assert "(no profile)" in caplog.records[0].message + + +class TestRoutingConsultation: + """Tests that _profile_name_for_source is consulted when source.profile is empty.""" + + def test_routing_consulted_when_source_profile_empty(self, mock_runner, discord_source): + """_profile_name_for_source should be called when source.profile is empty.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/routed") + + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + mock_runner._resolve_profile_home_for_source(discord_source) + + # Should have called routing + mock_runner._profile_name_for_source.assert_called_once_with(discord_source) + + def test_routing_not_consulted_when_source_profile_set(self, mock_runner, discord_source): + """_profile_name_for_source should NOT be called when source.profile is set.""" + discord_source.profile = "from-source" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/from-source") + + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + mock_runner._resolve_profile_home_for_source(discord_source) + + # Should NOT have called routing + mock_runner._profile_name_for_source.assert_not_called() diff --git a/tests/gateway/test_profile_routing.py b/tests/gateway/test_profile_routing.py index 865ec065f7e0..4df37a196955 100644 --- a/tests/gateway/test_profile_routing.py +++ b/tests/gateway/test_profile_routing.py @@ -215,3 +215,32 @@ class TestParentChatIdMatching: r = ProfileRoute(name="g", platform="discord", profile="server", guild_id="111") assert r.matches("discord", guild_id="111", chat_id="333", parent_chat_id="444") + + +class TestForumPostMatching: + """Test that forum posts match via parent_chat_id (direct parent).""" + + def test_forum_channel_route_matches_forum_post(self): + """A route on a forum channel should match comments on posts in that forum. + + In Discord, forum posts (threads) have parent_chat_id = forum channel ID. + No cache is needed — the parent relationship is direct. + """ + r = ProfileRoute(name="forum", platform="discord", profile="forum_profile", + chat_id="forum_channel_123") + # A comment on a forum post: chat_id=post_thread_id, parent_chat_id=forum_channel_id + assert r.matches("discord", chat_id="post_thread_456", parent_chat_id="forum_channel_123") + + def test_forum_post_comment_matches_channel_not_thread_id(self): + """Verify that thread_id matching is distinct from parent_chat_id matching.""" + routes = [ + ProfileRoute(name="forum", platform="discord", profile="forum_profile", + chat_id="forum_channel_123"), + ProfileRoute(name="post", platform="discord", profile="post_profile", + thread_id="post_thread_456"), + ] + # A comment on the forum post should match the forum channel route, not the thread route + m = match_profile_route(routes, "discord", chat_id="post_thread_456", + parent_chat_id="forum_channel_123") + assert m is not None + assert m.profile == "forum_profile"