From 025fc7e74b482b022607871881805cdbde4b9959 Mon Sep 17 00:00:00 2001 From: kyssta-exe Date: Fri, 7 Aug 2026 00:50:46 +0000 Subject: [PATCH] fix(slack): dedupe thread-qualified channel lookups (#80668) --- gateway/channel_directory.py | 36 ++++++++++++++++++------- tests/gateway/test_channel_directory.py | 22 +++++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/gateway/channel_directory.py b/gateway/channel_directory.py index 5a68a570a2b78..1ffe6d6866771 100644 --- a/gateway/channel_directory.py +++ b/gateway/channel_directory.py @@ -355,48 +355,64 @@ async def _build_slack(adapter) -> List[Dict[str, Any]]: continue # Merge in DM/group entries discovered from session history. + # Thread-qualified IDs are internal routing keys, not Slack API IDs. + def slack_lookup_id(entry_id: str) -> str: + return entry_id.split(":", 1)[0] + # Build a lookup from API-discovered channels so we can enrich session entries. api_name_lookup = {ch["id"]: ch["name"] for ch in channels} for entry in await asyncio.to_thread(_build_from_sessions, "slack"): eid = entry.get("id") + if not isinstance(eid, str): + continue if eid not in seen_ids: # If the entry name is still a raw Slack ID (e.g. C0xxx / D0xxx), - # try to resolve it from the API lookup first. + # try to resolve it from the API lookup using the base conversation ID. if entry.get("name", "").startswith(("C0", "D0", "G0")): - if eid in api_name_lookup: - entry["name"] = api_name_lookup[eid] + base_id = slack_lookup_id(eid) + if base_id in api_name_lookup: + entry["name"] = api_name_lookup[base_id] channels.append(entry) seen_ids.add(eid) # Resolve remaining raw-ID entries (DMs, private channels not in bot scope) - # by calling conversations.info + users.info for each. + # by calling conversations.info + users.info once per base conversation. unresolved = [ch for ch in channels if ch.get("name", "").startswith(("C0", "D0", "G0"))] if unresolved and team_clients: client = next(iter(team_clients.values())) + unresolved_by_base = {} for entry in unresolved: + unresolved_by_base.setdefault(slack_lookup_id(entry["id"]), []).append(entry) + for base_id, entries in unresolved_by_base.items(): try: - resp = await client.conversations_info(channel=entry["id"]) + resp = await client.conversations_info(channel=base_id) if not resp.get("ok"): continue ch_info = resp.get("channel", {}) + resolved_name = None + resolved_type = None if ch_info.get("is_im"): peer_user = ch_info.get("user", "") if peer_user: user_resp = await client.users_info(user=peer_user) if user_resp.get("ok"): u = user_resp["user"] - entry["name"] = ( + resolved_name = ( u.get("profile", {}).get("display_name") or u.get("real_name") or u.get("name") - or entry["id"] ) - entry["type"] = "dm" + resolved_type = "dm" else: - entry["name"] = ch_info.get("name") or ch_info.get("name_normalized") or entry["id"] + resolved_name = ch_info.get("name") or ch_info.get("name_normalized") + if resolved_name: + for entry in entries: + entry["name"] = resolved_name + if resolved_type: + entry["type"] = resolved_type except Exception as e: - logger.debug("Channel directory: failed to resolve %s: %s", entry["id"], e) + logger.debug("Channel directory: failed to resolve %s: %s", base_id, e) continue return channels diff --git a/tests/gateway/test_channel_directory.py b/tests/gateway/test_channel_directory.py index ce97ab7095cc9..6c7e651322c08 100644 --- a/tests/gateway/test_channel_directory.py +++ b/tests/gateway/test_channel_directory.py @@ -316,6 +316,28 @@ class TestBuildSlack: assert {e["id"] for e in entries} == {"C001", "C002"} assert client.users_conversations.await_count == 2 + def test_thread_ids_use_base_conversation_and_dedupe_info_calls(self, tmp_path, monkeypatch): + client = _make_slack_client([{"ok": True, "channels": [], "response_metadata": {}}]) + client.conversations_info = AsyncMock(side_effect=[ + {"ok": True, "channel": {"name": "engineering"}}, + {"ok": True, "channel": {"name": "support"}}, + ]) + monkeypatch.setattr( + "gateway.channel_directory._build_from_sessions", + lambda platform: [ + {"id": "C001:111", "name": "C001:111", "type": "channel"}, + {"id": "C001:222", "name": "C001:222", "type": "channel"}, + {"id": "C002:333", "name": "C002:333", "type": "channel"}, + ], + ) + + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + entries = asyncio.run(_build_slack(_make_slack_adapter({"T1": client}))) + + assert {entry["name"] for entry in entries} == {"engineering", "support"} + assert client.conversations_info.await_count == 2 + assert [call.kwargs["channel"] for call in client.conversations_info.await_args_list] == ["C001", "C002"] + class TestChannelAliases: """The user-maintained alias overlay (channel_aliases.json) gives durable