Skip to content

Commit 2b514b9

Browse files
committed
fix(slack): dedup channel mentions and make sender-contact creation idempotent
A channel @mention is delivered twice (app_mention + message) with distinct event_ids but the same client_msg_id; dedup now keys on the stable message id so the pair collapses to one processed event. _create_slack_contact pre-checks and, on a lost unique-slack_user_id race, resolves to the existing contact instead of dropping the message.
1 parent 99d94e1 commit 2b514b9

1 file changed

Lines changed: 44 additions & 11 deletions

File tree

unity/conversation_manager/comms_manager.py

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -108,23 +108,27 @@ def _already_seen_teams(message_id: str) -> bool:
108108
return False
109109

110110

111-
# In-memory dedup for Slack ``event_id``s. Slack's Events API may redeliver
112-
# events when an ack is missed within ~3s; the canonical recommendation is to
113-
# dedup on ``event_id`` for a few minutes.
111+
# In-memory dedup for inbound Slack messages. Two redelivery sources exist:
112+
# (1) the Events API replays an event when an ack is missed within ~3s, and
113+
# (2) a single channel mention is delivered twice -- once as ``app_mention``
114+
# and once as ``message`` -- with *distinct* ``event_id``s but the *same*
115+
# ``client_msg_id``. Keying on the stable message identity (client_msg_id,
116+
# falling back to ts) collapses both cases; keying on ``event_id`` would miss
117+
# the app_mention/message pair.
114118
_seen_slack_ids: dict[str, float] = {}
115119
_SLACK_DEDUP_TTL = 300.0
116120

117121

118-
def _already_seen_slack(event_id: str) -> bool:
119-
"""Return True if this Slack event_id was already processed recently."""
122+
def _already_seen_slack(message_key: str) -> bool:
123+
"""Return True if this Slack message key was already processed recently."""
120124
now = time.time()
121125
cutoff = now - _SLACK_DEDUP_TTL
122126
expired = [k for k, t in _seen_slack_ids.items() if t < cutoff]
123127
for k in expired:
124128
del _seen_slack_ids[k]
125-
if event_id in _seen_slack_ids:
129+
if message_key in _seen_slack_ids:
126130
return True
127-
_seen_slack_ids[event_id] = now
131+
_seen_slack_ids[message_key] = now
128132
return False
129133

130134

@@ -409,8 +413,27 @@ def _create_slack_contact(slack_user_id: str, profile: dict) -> dict | None:
409413
"""
410414
from unity.manager_registry import ManagerRegistry
411415

416+
cm = ManagerRegistry.get_contact_manager()
417+
418+
def _existing_by_slack_id() -> dict | None:
419+
result = cm.filter_contacts(
420+
filter=f"slack_user_id == '{slack_user_id}'",
421+
limit=1,
422+
)
423+
existing = result.get("contacts", [])
424+
if not existing:
425+
return None
426+
c = existing[0]
427+
return c.model_dump() if hasattr(c, "model_dump") else c
428+
429+
# Another in-flight event (the app_mention/message pair) or a prior
430+
# session may already own this slack_user_id. Reuse it rather than
431+
# racing into a duplicate insert.
432+
found = _existing_by_slack_id()
433+
if found is not None:
434+
return found
435+
412436
try:
413-
cm = ManagerRegistry.get_contact_manager()
414437
full_name = (
415438
profile.get("real_name") or profile.get("display_name") or ""
416439
).strip()
@@ -425,6 +448,11 @@ def _create_slack_contact(slack_user_id: str, profile: dict) -> dict | None:
425448
new_id = outcome["details"]["contact_id"]
426449
return cm.get_contact_info(new_id).get(new_id)
427450
except Exception as e:
451+
# Lost a create race on the unique slack_user_id constraint — the
452+
# winner's row is now queryable, so resolve to it instead of dropping.
453+
found = _existing_by_slack_id()
454+
if found is not None:
455+
return found
428456
LOGGER.error(f"{DEFAULT_ICON} Error creating Slack contact: {e}")
429457
return None
430458

@@ -1151,7 +1179,6 @@ def _normalize_recipients(value):
11511179

11521180
if thread == "slack":
11531181
sender_slack_user_id = event.get("sender_slack_user_id", "")
1154-
event_id = event.get("event_id") or event.get("message_id", "")
11551182
is_channel = event.get("is_channel", False)
11561183
team_id = event.get("team_id", "")
11571184
channel_id = event.get("channel_id", "")
@@ -1162,9 +1189,15 @@ def _normalize_recipients(value):
11621189
attachments = event.get("attachments") or []
11631190
routing_metadata = event.get("routing_metadata") or {}
11641191

1165-
if event_id and _already_seen_slack(event_id):
1192+
# Dedup on the stable message identity (client_msg_id/ts,
1193+
# carried as ``message_id``) so the app_mention + message
1194+
# pair for one channel mention collapses to a single
1195+
# processed event. Fall back to ``event_id`` only when no
1196+
# message id is present.
1197+
dedup_key = message_id or event.get("event_id", "")
1198+
if dedup_key and _already_seen_slack(dedup_key):
11661199
LOGGER.debug(
1167-
f"{DEFAULT_ICON} Skipping duplicate Slack event {event_id}",
1200+
f"{DEFAULT_ICON} Skipping duplicate Slack message {dedup_key}",
11681201
)
11691202
ack_now()
11701203
return

0 commit comments

Comments
 (0)