Skip to content

Commit ef6d519

Browse files
committed
fix(dingtalk): isolate shared group sessions
1 parent 46dfac7 commit ef6d519

4 files changed

Lines changed: 115 additions & 6 deletions

File tree

src/qwenpaw/app/channels/dingtalk/channel.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
from .content_utils import (
7878
parse_data_url,
7979
session_param_from_webhook_url,
80+
shared_group_session_id_from_conversation_id,
8081
short_session_id_from_conversation_id,
8182
)
8283
from .handler import DingTalkChannelHandler
@@ -368,10 +369,13 @@ def resolve_session_id(
368369
sender_id: str,
369370
channel_meta: Optional[Dict[str, Any]] = None,
370371
) -> str:
371-
"""Session_id = short suffix of conversation_id for cron lookup."""
372+
"""Resolve session_id from conversation metadata."""
372373
meta = channel_meta or {}
373374
cid = meta.get("conversation_id")
374375
if cid:
376+
# Shared groups need full-ID entropy because user_id is "group".
377+
if meta.get("is_group") and self.share_session_in_group:
378+
return shared_group_session_id_from_conversation_id(cid)
375379
return short_session_id_from_conversation_id(cid)
376380
return f"{self.channel}:{sender_id}"
377381

@@ -2503,12 +2507,12 @@ def _merge_native(self, items: list) -> dict:
25032507
payload = it if isinstance(it, dict) else {}
25042508
merged_parts.extend(payload.get("content_parts") or [])
25052509
m = payload.get("meta") or {}
2510+
# Keep first sender identity; refresh only delivery metadata.
25062511
for k in (
25072512
"conversation_id",
25082513
"session_webhook",
25092514
"session_webhook_expired_time",
25102515
"conversation_type",
2511-
"sender_staff_id",
25122516
):
25132517
if k in m:
25142518
merged_meta[k] = m[k]

src/qwenpaw/app/channels/dingtalk/constants.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
# Short suffix length for session_id from conversation_id
88
DINGTALK_SESSION_ID_SUFFIX_LEN = 8
99

10+
# Shared-group session IDs use 64 bits from SHA-256.
11+
DINGTALK_SHARED_SESSION_HASH_LEN = 16
12+
1013
# DingTalk message type to runtime content type
1114
DINGTALK_TYPE_MAPPING = {
1215
"picture": "image",

src/qwenpaw/app/channels/dingtalk/content_utils.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import base64
77
import binascii
8+
import hashlib
89
import re
910
from typing import Any, Optional
1011
from urllib.parse import parse_qs, urlparse
@@ -20,6 +21,7 @@
2021

2122
from .constants import (
2223
DINGTALK_SESSION_ID_SUFFIX_LEN,
24+
DINGTALK_SHARED_SESSION_HASH_LEN,
2325
DINGTALK_TYPE_MAPPING,
2426
)
2527

@@ -126,6 +128,14 @@ def short_session_id_from_conversation_id(conversation_id: str) -> str:
126128
)
127129

128130

131+
def shared_group_session_id_from_conversation_id(
132+
conversation_id: str,
133+
) -> str:
134+
"""Hash a group conversation ID into a short stable session ID."""
135+
digest = hashlib.sha256(conversation_id.encode("utf-8")).hexdigest()
136+
return digest[:DINGTALK_SHARED_SESSION_HASH_LEN]
137+
138+
129139
def session_param_from_webhook_url(url: str) -> Optional[str]:
130140
"""Extract session= param from sendBySession URL for debug logging."""
131141
if not url or "?" not in url:

tests/unit/channels/test_dingtalk.py

Lines changed: 96 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,15 @@ def test_route_from_handle_empty(self, dingtalk_channel):
985985
# =============================================================================
986986

987987

988+
def _shared_sid(conversation_id: str) -> str:
989+
"""Expected shared-group session_id for a conversation_id."""
990+
from qwenpaw.app.channels.dingtalk.content_utils import (
991+
shared_group_session_id_from_conversation_id,
992+
)
993+
994+
return shared_group_session_id_from_conversation_id(conversation_id)
995+
996+
988997
class TestDingTalkShareSessionInGroup:
989998
"""Tests for per-user vs shared context in group chats."""
990999

@@ -1040,7 +1049,8 @@ def test_group_shared_collapses_user_id(
10401049
)
10411050

10421051
assert request.user_id == "group"
1043-
assert request.session_id == "Y7890XYZ"
1052+
assert request.session_id == _shared_sid("cidQWERTY7890XYZ")
1053+
assert request.session_id != "Y7890XYZ"
10441054

10451055
def test_shared_user_id_has_no_underscore(
10461056
self,
@@ -1056,9 +1066,10 @@ def test_shared_user_id_has_no_underscore(
10561066
session_id=request.session_id,
10571067
)
10581068

1059-
assert to_handle == "dingtalk:sw:group_Y7890XYZ"
1069+
sid = _shared_sid("cidQWERTY7890XYZ")
1070+
assert to_handle == f"dingtalk:sw:group_{sid}"
10601071
fallback = channel._suffix_only_webhook_key(to_handle)
1061-
assert fallback == "dingtalk:sw:Y7890XYZ"
1072+
assert fallback == f"dingtalk:sw:{sid}"
10621073

10631074
def test_dm_unaffected_by_sharing(self, dingtalk_channel_shared_group):
10641075
"""Direct messages keep their own user_id when sharing is on."""
@@ -1082,7 +1093,7 @@ def test_debounce_key_shared_drops_sender(
10821093
self._group_payload(),
10831094
)
10841095

1085-
assert key == "Y7890XYZ"
1096+
assert key == _shared_sid("cidQWERTY7890XYZ")
10861097

10871098
def test_debounce_key_shared_dm_keeps_sender(
10881099
self,
@@ -1113,6 +1124,87 @@ def test_from_config_passes_flag(self, mock_process_handler):
11131124

11141125
assert channel.share_session_in_group is True
11151126

1127+
def test_merge_two_members_keeps_first_sender_identity(
1128+
self,
1129+
dingtalk_channel_shared_group,
1130+
):
1131+
"""Merging members must not mix sender identity fields."""
1132+
channel = dingtalk_channel_shared_group
1133+
alice = self._group_payload()
1134+
alice["meta"]["sender_staff_id"] = "staff_alice"
1135+
alice["meta"]["user_name"] = "Alice"
1136+
bob = self._group_payload()
1137+
bob["sender_id"] = "Bob#5678"
1138+
bob["acl_sender_id"] = "staff_bob"
1139+
bob["meta"]["sender_staff_id"] = "staff_bob"
1140+
bob["meta"]["user_name"] = "Bob"
1141+
1142+
merged = channel.merge_native_items([alice, bob])
1143+
1144+
assert merged["sender_id"] == "Alice#1234"
1145+
assert merged["acl_sender_id"] == "staff_alice"
1146+
assert merged["meta"]["user_name"] == "Alice"
1147+
assert merged["meta"]["sender_staff_id"] == "staff_alice"
1148+
1149+
def test_merge_two_members_tracks_newest_session(
1150+
self,
1151+
dingtalk_channel_shared_group,
1152+
):
1153+
"""Conversation/webhook state still follows the newest item."""
1154+
channel = dingtalk_channel_shared_group
1155+
alice = self._group_payload()
1156+
alice["meta"]["session_webhook"] = "https://old.example"
1157+
bob = self._group_payload()
1158+
bob["sender_id"] = "Bob#5678"
1159+
bob["meta"]["session_webhook"] = "https://new.example"
1160+
1161+
merged = channel.merge_native_items([alice, bob])
1162+
1163+
assert merged["meta"]["session_webhook"] == "https://new.example"
1164+
assert merged["meta"]["batched_count"] == 2
1165+
1166+
def test_shared_groups_with_same_suffix_stay_separate(
1167+
self,
1168+
dingtalk_channel_shared_group,
1169+
):
1170+
"""Groups sharing an 8-char suffix stay isolated."""
1171+
channel = dingtalk_channel_shared_group
1172+
first = self._group_payload()
1173+
first["meta"]["conversation_id"] = "cidAAAASAME8888"
1174+
second = self._group_payload()
1175+
second["meta"]["conversation_id"] = "cidBBBBSAME8888"
1176+
1177+
req_a = channel.build_agent_request_from_native(first)
1178+
req_b = channel.build_agent_request_from_native(second)
1179+
1180+
assert first["meta"]["conversation_id"][-8:] == (
1181+
second["meta"]["conversation_id"][-8:]
1182+
)
1183+
assert req_a.session_id != req_b.session_id
1184+
assert channel.get_debounce_key(first) != channel.get_debounce_key(
1185+
second,
1186+
)
1187+
1188+
def test_shared_session_id_is_underscore_free(
1189+
self,
1190+
dingtalk_channel_shared_group,
1191+
):
1192+
"""Hash must stay "_"-free for the webhook fallback key split."""
1193+
channel = dingtalk_channel_shared_group
1194+
request = channel.build_agent_request_from_native(
1195+
self._group_payload(),
1196+
)
1197+
1198+
assert "_" not in request.session_id
1199+
1200+
def test_isolated_mode_keeps_short_suffix(self, dingtalk_channel):
1201+
"""Isolated mode keeps the legacy suffix (no state migration)."""
1202+
request = dingtalk_channel.build_agent_request_from_native(
1203+
self._group_payload(),
1204+
)
1205+
1206+
assert request.session_id == "Y7890XYZ"
1207+
11161208

11171209
# =============================================================================
11181210
# P2: Open API Fallback (Critical for CHAN-D02)

0 commit comments

Comments
 (0)