mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-17 09:41:58 +00:00
fix(messaging): route WhatsApp group JIDs to the target, not the home DM
send_message(target="whatsapp:<group-jid>") silently delivered to the configured home DM instead of the requested group. Two gaps: 1. _parse_target_ref had no WhatsApp branch. Group JIDs (<id>@g.us), user JIDs (<id>@s.whatsapp.net), linked-identity JIDs (<id>@lid), and broadcast/newsletter JIDs matched no pattern and fell through to `return None, None, False`, so the caller treated them as unresolvable and used the home channel. The bridge's /send endpoint accepts any chatId, so only the tool-side target parsing was at fault. Add a whatsapp branch that recognizes native JIDs as explicit targets. The pre-existing '+'-prefixed E.164 path is preserved. 2. WhatsApp groups have no human-friendly name — the channel directory is regenerated from session data on a timer, so a group shows up as its raw 18-digit JID and any hand-edit to channel_directory.json is clobbered on the next rebuild. Add a user-maintained alias overlay (~/.hermes/channel_aliases.json) re-applied on every build AND every load, giving durable friendly names and letting a freshly-created group be pre-named before its first message. Tests: TestParseTargetRefWhatsAppJID (7 cases) for the parser; TestChannelAliases (7 cases) for the overlay, plus an autouse fixture isolating CHANNEL_ALIASES_PATH so a real alias file can't leak into the existing directory tests.
This commit is contained in:
parent
c17469cb19
commit
ea49a79633
4 changed files with 219 additions and 3 deletions
|
|
@ -17,6 +17,53 @@ from utils import atomic_json_write
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
DIRECTORY_PATH = get_hermes_home() / "channel_directory.json"
|
||||
# User-maintained friendly-name overlay. The directory is fully regenerated
|
||||
# from live adapters + session data on a timer, so hand-edits to
|
||||
# channel_directory.json don't survive. Aliases declared here are re-applied
|
||||
# on every build AND every load, giving durable human-friendly names (and
|
||||
# letting you pre-name a chat before it has produced any traffic).
|
||||
# Format: {"<platform>": {"<chat_id>": "<friendly name>", ...}, ...}
|
||||
CHANNEL_ALIASES_PATH = get_hermes_home() / "channel_aliases.json"
|
||||
|
||||
|
||||
def _load_channel_aliases() -> Dict[str, Dict[str, str]]:
|
||||
if not CHANNEL_ALIASES_PATH.exists():
|
||||
return {}
|
||||
try:
|
||||
with open(CHANNEL_ALIASES_PATH, encoding="utf-8") as f:
|
||||
data = json.load(f)
|
||||
return data if isinstance(data, dict) else {}
|
||||
except Exception:
|
||||
return {}
|
||||
|
||||
|
||||
def _apply_channel_aliases(platforms: Dict[str, Any]) -> None:
|
||||
"""Overlay friendly names onto directory entries by chat_id.
|
||||
|
||||
Renames matching entries in place; injects a placeholder entry for an
|
||||
aliased id that hasn't been discovered yet (so a freshly-created group is
|
||||
addressable by name before its first message). Mutates *platforms*.
|
||||
"""
|
||||
aliases = _load_channel_aliases()
|
||||
for plat_name, id_map in aliases.items():
|
||||
if not isinstance(id_map, dict):
|
||||
continue
|
||||
entries = platforms.setdefault(plat_name, [])
|
||||
for chat_id, friendly in id_map.items():
|
||||
if not friendly:
|
||||
continue
|
||||
matched = False
|
||||
for e in entries:
|
||||
if e.get("id") == chat_id:
|
||||
e["name"] = friendly
|
||||
matched = True
|
||||
if not matched:
|
||||
entries.append({
|
||||
"id": chat_id,
|
||||
"name": friendly,
|
||||
"type": "group" if str(chat_id).endswith("@g.us") else "dm",
|
||||
"thread_id": None,
|
||||
})
|
||||
|
||||
|
||||
def _normalize_channel_query(value: str) -> str:
|
||||
|
|
@ -96,6 +143,9 @@ async def build_channel_directory(adapters: Dict[Any, Any]) -> Dict[str, Any]:
|
|||
except Exception:
|
||||
pass
|
||||
|
||||
# Overlay user-maintained friendly names before persisting.
|
||||
_apply_channel_aliases(platforms)
|
||||
|
||||
directory = {
|
||||
"updated_at": datetime.now().isoformat(),
|
||||
"platforms": platforms,
|
||||
|
|
@ -247,12 +297,20 @@ def _build_from_sessions(platform_name: str) -> List[Dict[str, str]]:
|
|||
def load_directory() -> Dict[str, Any]:
|
||||
"""Load the cached channel directory from disk."""
|
||||
if not DIRECTORY_PATH.exists():
|
||||
return {"updated_at": None, "platforms": {}}
|
||||
base = {"updated_at": None, "platforms": {}}
|
||||
_apply_channel_aliases(base["platforms"])
|
||||
return base
|
||||
try:
|
||||
with open(DIRECTORY_PATH, encoding="utf-8") as f:
|
||||
return json.load(f)
|
||||
data = json.load(f)
|
||||
# Re-apply aliases on read so friendly names take effect immediately,
|
||||
# even between timed rebuilds and for brand-new alias entries.
|
||||
_apply_channel_aliases(data.setdefault("platforms", {}))
|
||||
return data
|
||||
except Exception:
|
||||
return {"updated_at": None, "platforms": {}}
|
||||
base = {"updated_at": None, "platforms": {}}
|
||||
_apply_channel_aliases(base["platforms"])
|
||||
return base
|
||||
|
||||
|
||||
def lookup_channel_type(platform_name: str, chat_id: str) -> Optional[str]:
|
||||
|
|
|
|||
|
|
@ -12,11 +12,26 @@ from gateway.channel_directory import (
|
|||
resolve_channel_name,
|
||||
format_directory_for_display,
|
||||
load_directory,
|
||||
_apply_channel_aliases,
|
||||
_build_from_sessions,
|
||||
_build_slack,
|
||||
)
|
||||
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_channel_aliases(tmp_path_factory):
|
||||
"""Point the alias overlay at a nonexistent path by default so a real
|
||||
~/.hermes/channel_aliases.json never leaks into directory tests. Tests
|
||||
that exercise aliases patch CHANNEL_ALIASES_PATH themselves inside the
|
||||
test body, which takes precedence over this outer patch."""
|
||||
missing = tmp_path_factory.mktemp("aliases") / "none.json"
|
||||
with patch("gateway.channel_directory.CHANNEL_ALIASES_PATH", missing):
|
||||
yield
|
||||
|
||||
|
||||
def _write_directory(tmp_path, platforms):
|
||||
"""Helper to write a fake channel directory."""
|
||||
data = {"updated_at": "2026-01-01T00:00:00", "platforms": platforms}
|
||||
|
|
@ -480,3 +495,80 @@ class TestBuildSlack:
|
|||
entries = asyncio.run(_build_slack(_make_slack_adapter({"T1": client})))
|
||||
|
||||
assert entries == []
|
||||
|
||||
|
||||
class TestChannelAliases:
|
||||
"""The user-maintained alias overlay (channel_aliases.json) gives durable
|
||||
friendly names that survive the timed directory rebuild."""
|
||||
|
||||
def _setup_aliases(self, tmp_path, aliases):
|
||||
alias_file = tmp_path / "channel_aliases.json"
|
||||
alias_file.write_text(json.dumps(aliases))
|
||||
return patch("gateway.channel_directory.CHANNEL_ALIASES_PATH", alias_file)
|
||||
|
||||
def test_alias_renames_existing_entry_on_load(self, tmp_path):
|
||||
cache_file = _write_directory(tmp_path, {
|
||||
"whatsapp": [{"id": "120363@g.us", "name": "120363", "type": "group"}]
|
||||
})
|
||||
with patch("gateway.channel_directory.DIRECTORY_PATH", cache_file), \
|
||||
self._setup_aliases(tmp_path, {"whatsapp": {"120363@g.us": "general"}}):
|
||||
result = load_directory()
|
||||
assert result["platforms"]["whatsapp"][0]["name"] == "general"
|
||||
# And the friendly name resolves back to the JID
|
||||
assert resolve_channel_name("whatsapp", "general") == "120363@g.us"
|
||||
assert resolve_channel_name("whatsapp", "GENERAL") == "120363@g.us"
|
||||
|
||||
def test_alias_injects_undiscovered_group(self, tmp_path):
|
||||
"""A group named in the alias file but not yet seen in any session is
|
||||
still addressable by name (pre-naming before first traffic)."""
|
||||
cache_file = _write_directory(tmp_path, {"whatsapp": []})
|
||||
with patch("gateway.channel_directory.DIRECTORY_PATH", cache_file), \
|
||||
self._setup_aliases(tmp_path, {"whatsapp": {"999@g.us": "marketing"}}):
|
||||
assert resolve_channel_name("whatsapp", "marketing") == "999@g.us"
|
||||
entries = load_directory()["platforms"]["whatsapp"]
|
||||
injected = [e for e in entries if e["id"] == "999@g.us"]
|
||||
assert injected and injected[0]["type"] == "group"
|
||||
|
||||
def test_no_alias_file_is_noop(self, tmp_path):
|
||||
cache_file = _write_directory(tmp_path, {
|
||||
"whatsapp": [{"id": "120363@g.us", "name": "120363", "type": "group"}]
|
||||
})
|
||||
with patch("gateway.channel_directory.DIRECTORY_PATH", cache_file), \
|
||||
patch("gateway.channel_directory.CHANNEL_ALIASES_PATH", tmp_path / "nope.json"):
|
||||
result = load_directory()
|
||||
assert result["platforms"]["whatsapp"][0]["name"] == "120363"
|
||||
|
||||
def test_corrupt_alias_file_is_ignored(self, tmp_path):
|
||||
cache_file = _write_directory(tmp_path, {
|
||||
"whatsapp": [{"id": "120363@g.us", "name": "120363", "type": "group"}]
|
||||
})
|
||||
bad = tmp_path / "channel_aliases.json"
|
||||
bad.write_text("{not json")
|
||||
with patch("gateway.channel_directory.DIRECTORY_PATH", cache_file), \
|
||||
patch("gateway.channel_directory.CHANNEL_ALIASES_PATH", bad):
|
||||
result = load_directory()
|
||||
assert result["platforms"]["whatsapp"][0]["name"] == "120363"
|
||||
|
||||
def test_alias_persists_through_rebuild(self, tmp_path, monkeypatch):
|
||||
"""build_channel_directory must bake aliases into the written file so
|
||||
they survive the periodic regeneration, not just live reads."""
|
||||
cache_file = tmp_path / "channel_directory.json"
|
||||
monkeypatch.setattr("gateway.channel_directory._build_from_sessions",
|
||||
lambda plat: [{"id": "120363@g.us", "name": "120363",
|
||||
"type": "group", "thread_id": None}]
|
||||
if plat == "whatsapp" else [])
|
||||
with patch("gateway.channel_directory.DIRECTORY_PATH", cache_file), \
|
||||
self._setup_aliases(tmp_path, {"whatsapp": {"120363@g.us": "general"}}):
|
||||
asyncio.run(build_channel_directory({}))
|
||||
on_disk = json.loads(cache_file.read_text())
|
||||
names = [e["name"] for e in on_disk["platforms"]["whatsapp"]
|
||||
if e["id"] == "120363@g.us"]
|
||||
assert names == ["general"]
|
||||
|
||||
def test_apply_aliases_handles_malformed_map(self):
|
||||
"""Non-dict alias entries must not raise."""
|
||||
platforms = {"whatsapp": [{"id": "1@g.us", "name": "1", "type": "group"}]}
|
||||
with patch("gateway.channel_directory._load_channel_aliases",
|
||||
return_value={"whatsapp": "not-a-dict", "telegram": None}):
|
||||
_apply_channel_aliases(platforms) # should not raise
|
||||
assert platforms["whatsapp"][0]["name"] == "1"
|
||||
|
|
|
|||
|
|
@ -1224,6 +1224,58 @@ class TestParseTargetRefE164:
|
|||
assert _parse_target_ref("matrix", "+15551234567")[2] is False
|
||||
|
||||
|
||||
class TestParseTargetRefWhatsAppJID:
|
||||
"""_parse_target_ref accepts native WhatsApp JIDs as explicit targets.
|
||||
|
||||
Regression: group JIDs (``<id>@g.us``) and linked-identity JIDs
|
||||
(``<id>@lid``) matched no branch and fell through to home-channel
|
||||
resolution, so ``send_message(target="whatsapp:<group-jid>")`` silently
|
||||
delivered to the configured home DM instead of the requested group.
|
||||
"""
|
||||
|
||||
def test_group_jid_is_explicit(self):
|
||||
chat_id, thread_id, is_explicit = _parse_target_ref(
|
||||
"whatsapp", "120363408391911677@g.us"
|
||||
)
|
||||
assert chat_id == "120363408391911677@g.us"
|
||||
assert thread_id is None
|
||||
assert is_explicit is True
|
||||
|
||||
def test_user_jid_is_explicit(self):
|
||||
chat_id, _, is_explicit = _parse_target_ref(
|
||||
"whatsapp", "19255551234@s.whatsapp.net"
|
||||
)
|
||||
assert chat_id == "19255551234@s.whatsapp.net"
|
||||
assert is_explicit is True
|
||||
|
||||
def test_lid_jid_is_explicit(self):
|
||||
chat_id, _, is_explicit = _parse_target_ref(
|
||||
"whatsapp", "149606612619433@lid"
|
||||
)
|
||||
assert chat_id == "149606612619433@lid"
|
||||
assert is_explicit is True
|
||||
|
||||
def test_broadcast_and_newsletter_jids_are_explicit(self):
|
||||
assert _parse_target_ref("whatsapp", "status@broadcast")[2] is True
|
||||
assert _parse_target_ref("whatsapp", "120363000000000000@newsletter")[2] is True
|
||||
|
||||
def test_whatsapp_e164_still_explicit_alongside_jids(self):
|
||||
"""The pre-existing '+'-prefixed E.164 path must keep working."""
|
||||
chat_id, _, is_explicit = _parse_target_ref("whatsapp", "+15551234567")
|
||||
assert chat_id == "+15551234567"
|
||||
assert is_explicit is True
|
||||
|
||||
def test_jid_suffix_only_matches_whatsapp(self):
|
||||
"""WhatsApp JID suffixes must NOT be treated as explicit elsewhere."""
|
||||
assert _parse_target_ref("telegram", "120363408391911677@g.us")[2] is False
|
||||
assert _parse_target_ref("signal", "149606612619433@lid")[2] is False
|
||||
|
||||
def test_non_jid_whatsapp_target_falls_through(self):
|
||||
"""A bare friendly name is not a JID — it must fall through to
|
||||
directory resolution (returns not-explicit so the caller can resolve)."""
|
||||
assert _parse_target_ref("whatsapp", "general")[2] is False
|
||||
|
||||
|
||||
class TestParseTargetRefSlack:
|
||||
"""_parse_target_ref recognizes Slack channel/user IDs as explicit."""
|
||||
|
||||
|
|
|
|||
|
|
@ -40,6 +40,14 @@ _NUMERIC_TOPIC_RE = _TELEGRAM_TOPIC_TARGET_RE
|
|||
# downstream adapters (signal, etc.) expect.
|
||||
_PHONE_PLATFORMS = frozenset({"photon", "signal", "sms", "whatsapp"})
|
||||
_E164_TARGET_RE = re.compile(r"^\s*\+(\d{7,15})\s*$")
|
||||
# WhatsApp JIDs: group chats (<digits>@g.us), individual users
|
||||
# (<phone>@s.whatsapp.net), linked identities (<id>@lid), and broadcast /
|
||||
# newsletter chats. These are explicit native targets the bridge accepts
|
||||
# verbatim — they must NOT fall through to home-channel resolution.
|
||||
_WHATSAPP_JID_RE = re.compile(
|
||||
r"^\s*[\w-]+@(?:g\.us|s\.whatsapp\.net|lid|broadcast|newsletter)\s*$",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
# Email addresses — a valid email like "user@domain.com" should be treated as
|
||||
# an explicit target for the email platform, not fall through to channel-name
|
||||
# resolution which has no way to resolve a raw address.
|
||||
|
|
@ -509,6 +517,12 @@ def _parse_target_ref(platform_name: str, target_ref: str):
|
|||
match = _EMAIL_TARGET_RE.fullmatch(target_ref)
|
||||
if match:
|
||||
return target_ref.strip(), None, True
|
||||
if platform_name == "whatsapp":
|
||||
# Native WhatsApp JIDs (group @g.us, user @s.whatsapp.net, @lid, etc.)
|
||||
# are explicit targets — pass through verbatim. E.164 '+' numbers fall
|
||||
# through to the _PHONE_PLATFORMS handler below.
|
||||
if _WHATSAPP_JID_RE.fullmatch(target_ref):
|
||||
return target_ref.strip(), None, True
|
||||
if platform_name in _PHONE_PLATFORMS:
|
||||
match = _E164_TARGET_RE.fullmatch(target_ref)
|
||||
if match:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue