fix(send_message): preserve Slack and Matrix thread targets resolved from channel directory
This commit is contained in:
@@ -182,6 +182,81 @@ class TestSendMessageTool:
|
|||||||
force_document=False,
|
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):
|
def test_mirror_receives_current_session_user_id(self):
|
||||||
config, _telegram_cfg = _make_config()
|
config, _telegram_cfg = _make_config()
|
||||||
|
|
||||||
@@ -503,9 +578,8 @@ class TestSendToPlatformChunking:
|
|||||||
assert all(call == [] for call in sent_calls[:-1])
|
assert all(call == [] for call in sent_calls[:-1])
|
||||||
assert sent_calls[-1] == media
|
assert sent_calls[-1] == media
|
||||||
|
|
||||||
def test_matrix_media_uses_native_adapter_helper(self):
|
def test_matrix_media_uses_native_adapter_helper(self, tmp_path):
|
||||||
|
doc_path = tmp_path / "test-send-message-matrix.pdf"
|
||||||
doc_path = Path("/tmp/test-send-message-matrix.pdf")
|
|
||||||
doc_path.write_bytes(b"%PDF-1.4 test")
|
doc_path.write_bytes(b"%PDF-1.4 test")
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -847,6 +921,16 @@ class TestParseTargetRefDiscord:
|
|||||||
class TestParseTargetRefMatrix:
|
class TestParseTargetRefMatrix:
|
||||||
"""_parse_target_ref correctly handles Matrix room IDs and user MXIDs."""
|
"""_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):
|
def test_matrix_room_id_is_explicit(self):
|
||||||
"""Matrix room IDs (!) are recognized as explicit targets."""
|
"""Matrix room IDs (!) are recognized as explicit targets."""
|
||||||
chat_id, thread_id, is_explicit = _parse_target_ref("matrix", "!HLOQwxYGgFPMPJUSNR:matrix.org")
|
chat_id, thread_id, is_explicit = _parse_target_ref("matrix", "!HLOQwxYGgFPMPJUSNR:matrix.org")
|
||||||
@@ -919,6 +1003,12 @@ class TestParseTargetRefE164:
|
|||||||
class TestParseTargetRefSlack:
|
class TestParseTargetRefSlack:
|
||||||
"""_parse_target_ref recognizes Slack channel/user IDs as explicit."""
|
"""_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):
|
def test_public_channel_id_is_explicit(self):
|
||||||
chat_id, thread_id, is_explicit = _parse_target_ref("slack", "C0B0QV5434G")
|
chat_id, thread_id, is_explicit = _parse_target_ref("slack", "C0B0QV5434G")
|
||||||
assert chat_id == "C0B0QV5434G"
|
assert chat_id == "C0B0QV5434G"
|
||||||
|
|||||||
@@ -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
|
# 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.
|
# 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*$")
|
_SLACK_TARGET_RE = re.compile(r"^\s*([CGD][A-Z0-9]{8,})\s*$")
|
||||||
|
# Session-derived Slack thread targets use "<conversation_id>:<thread_ts>".
|
||||||
|
_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*$")
|
_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*$")
|
_YUANBAO_TARGET_RE = re.compile(r"^\s*((?:group|direct):[^:]+)\s*$")
|
||||||
# Discord snowflake IDs are numeric, same regex pattern as Telegram topic targets.
|
# 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:
|
if match:
|
||||||
return match.group(1), match.group(2), True
|
return match.group(1), match.group(2), True
|
||||||
if platform_name == "slack":
|
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)
|
match = _SLACK_TARGET_RE.fullmatch(target_ref)
|
||||||
if match:
|
if match:
|
||||||
return match.group(1), None, True
|
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":
|
if platform_name == "weixin":
|
||||||
match = _WEIXIN_TARGET_RE.fullmatch(target_ref)
|
match = _WEIXIN_TARGET_RE.fullmatch(target_ref)
|
||||||
if match:
|
if match:
|
||||||
|
|||||||
Reference in New Issue
Block a user