From 2f28b60a474c880367be612c682f52b8ca9dbb4d Mon Sep 17 00:00:00 2001 From: QuenVix <164776164+QuenVix@users.noreply.github.com> Date: Sat, 16 May 2026 08:26:41 +0300 Subject: [PATCH] fix(send_message): preserve Slack and Matrix thread targets resolved from channel directory --- tests/tools/test_send_message_tool.py | 96 ++++++++++++++++++++++++++- tools/send_message_tool.py | 10 +++ 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/tests/tools/test_send_message_tool.py b/tests/tools/test_send_message_tool.py index fa810eb5c5..dac476749f 100644 --- a/tests/tools/test_send_message_tool.py +++ b/tests/tools/test_send_message_tool.py @@ -182,6 +182,81 @@ class TestSendMessageTool: force_document=False, ) + def test_resolved_slack_thread_name_preserves_thread_id(self): + slack_cfg = SimpleNamespace(enabled=True, token="xoxb-test", extra={}) + config = SimpleNamespace( + platforms={Platform.SLACK: slack_cfg}, + get_home_channel=lambda _platform: None, + ) + + with patch("gateway.config.load_gateway_config", return_value=config), \ + patch("tools.interrupt.is_interrupted", return_value=False), \ + patch("gateway.channel_directory.resolve_channel_name", return_value="C123ABCDEF:171.000001"), \ + patch("model_tools._run_async", side_effect=_run_async_immediately), \ + patch("tools.send_message_tool._send_to_platform", new=AsyncMock(return_value={"success": True})) as send_mock, \ + patch("gateway.mirror.mirror_to_session", return_value=True): + result = json.loads( + send_message_tool( + { + "action": "send", + "target": "slack:ops / topic 171.000001", + "message": "hello", + } + ) + ) + + assert result["success"] is True + send_mock.assert_awaited_once_with( + Platform.SLACK, + slack_cfg, + "C123ABCDEF", + "hello", + thread_id="171.000001", + media_files=[], + force_document=False, + ) + + def test_resolved_matrix_thread_name_preserves_thread_id(self): + matrix_cfg = SimpleNamespace( + enabled=True, + token="tok", + extra={"homeserver": "https://matrix.example.com"}, + ) + config = SimpleNamespace( + platforms={Platform.MATRIX: matrix_cfg}, + get_home_channel=lambda _platform: None, + ) + + with patch("gateway.config.load_gateway_config", return_value=config), \ + patch("tools.interrupt.is_interrupted", return_value=False), \ + patch( + "gateway.channel_directory.resolve_channel_name", + return_value="!roomid:matrix.example.org:$thread123:matrix.example.org", + ), \ + patch("model_tools._run_async", side_effect=_run_async_immediately), \ + patch("tools.send_message_tool._send_to_platform", new=AsyncMock(return_value={"success": True})) as send_mock, \ + patch("gateway.mirror.mirror_to_session", return_value=True): + result = json.loads( + send_message_tool( + { + "action": "send", + "target": "matrix:Ops / topic $thread123", + "message": "hello", + } + ) + ) + + assert result["success"] is True + send_mock.assert_awaited_once_with( + Platform.MATRIX, + matrix_cfg, + "!roomid:matrix.example.org", + "hello", + thread_id="$thread123:matrix.example.org", + media_files=[], + force_document=False, + ) + def test_mirror_receives_current_session_user_id(self): config, _telegram_cfg = _make_config() @@ -503,9 +578,8 @@ class TestSendToPlatformChunking: assert all(call == [] for call in sent_calls[:-1]) assert sent_calls[-1] == media - def test_matrix_media_uses_native_adapter_helper(self): - - doc_path = Path("/tmp/test-send-message-matrix.pdf") + def test_matrix_media_uses_native_adapter_helper(self, tmp_path): + doc_path = tmp_path / "test-send-message-matrix.pdf" doc_path.write_bytes(b"%PDF-1.4 test") try: @@ -847,6 +921,16 @@ class TestParseTargetRefDiscord: class TestParseTargetRefMatrix: """_parse_target_ref correctly handles Matrix room IDs and user MXIDs.""" + def test_matrix_thread_target_is_explicit(self): + """Session-derived Matrix thread targets round-trip as room + event id.""" + chat_id, thread_id, is_explicit = _parse_target_ref( + "matrix", + "!HLOQwxYGgFPMPJUSNR:matrix.org:$thread123:matrix.org", + ) + assert chat_id == "!HLOQwxYGgFPMPJUSNR:matrix.org" + assert thread_id == "$thread123:matrix.org" + assert is_explicit is True + def test_matrix_room_id_is_explicit(self): """Matrix room IDs (!) are recognized as explicit targets.""" chat_id, thread_id, is_explicit = _parse_target_ref("matrix", "!HLOQwxYGgFPMPJUSNR:matrix.org") @@ -919,6 +1003,12 @@ class TestParseTargetRefE164: class TestParseTargetRefSlack: """_parse_target_ref recognizes Slack channel/user IDs as explicit.""" + def test_thread_target_is_explicit(self): + chat_id, thread_id, is_explicit = _parse_target_ref("slack", "C0B0QV5434G:171.000001") + assert chat_id == "C0B0QV5434G" + assert thread_id == "171.000001" + assert is_explicit is True + def test_public_channel_id_is_explicit(self): chat_id, thread_id, is_explicit = _parse_target_ref("slack", "C0B0QV5434G") assert chat_id == "C0B0QV5434G" diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index d5b2c0c782..bfe1a63070 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -28,6 +28,8 @@ _FEISHU_TARGET_RE = re.compile(r"^\s*((?:oc|ou|on|chat|open)_[-A-Za-z0-9]+)(?::( # conversations.open to obtain a D... ID. Without this gate, Slack IDs fall # through to channel-name resolution, which only matches by name and fails. _SLACK_TARGET_RE = re.compile(r"^\s*([CGD][A-Z0-9]{8,})\s*$") +# Session-derived Slack thread targets use ":". +_SLACK_THREAD_TARGET_RE = re.compile(r"^\s*([CGD][A-Z0-9]{8,}):([^\s:]+)\s*$") _WEIXIN_TARGET_RE = re.compile(r"^\s*((?:wxid|gh|v\d+|wm|wb)_[A-Za-z0-9_-]+|[A-Za-z0-9._-]+@chatroom|filehelper)\s*$") _YUANBAO_TARGET_RE = re.compile(r"^\s*((?:group|direct):[^:]+)\s*$") # Discord snowflake IDs are numeric, same regex pattern as Telegram topic targets. @@ -330,9 +332,17 @@ def _parse_target_ref(platform_name: str, target_ref: str): if match: return match.group(1), match.group(2), True if platform_name == "slack": + match = _SLACK_THREAD_TARGET_RE.fullmatch(target_ref) + if match: + return match.group(1), match.group(2), True match = _SLACK_TARGET_RE.fullmatch(target_ref) if match: return match.group(1), None, True + if platform_name == "matrix": + trimmed = target_ref.strip() + split_idx = trimmed.rfind(":$") + if split_idx > 0: + return trimmed[:split_idx], trimmed[split_idx + 1 :], True if platform_name == "weixin": match = _WEIXIN_TARGET_RE.fullmatch(target_ref) if match: