From 75d3eaa0e4b9c602933b2ad269f0ea0f593b5d2d Mon Sep 17 00:00:00 2001 From: bde3249023 Date: Sun, 26 Apr 2026 12:27:19 -0700 Subject: [PATCH] fix(slack): exclude U/W user IDs from explicit target regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack's chat.postMessage API rejects user IDs (U...) and workspace IDs (W...) — they are not valid conversation IDs. Posting to them fails because the API requires a channel ID (C/G/D). To DM a user, the sender must first call conversations.open to obtain a D... ID. Tighten _SLACK_TARGET_RE from [CGDUW] to [CGD] so the send path rejects U/W values as explicit targets and instead falls through to channel- name resolution (where they'll fail with a clear 'could not resolve' error rather than silently getting stuck in a retry loop on the API). Flip the corresponding regression test to assert U/W values are not explicit. Matches the narrower regex briandevans proposed in #15939. Co-authored-by: briandevans --- tests/tools/test_send_message_tool.py | 10 +++++++--- tools/send_message_tool.py | 11 +++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/tools/test_send_message_tool.py b/tests/tools/test_send_message_tool.py index 60f71af69d..3fc08b31e3 100644 --- a/tests/tools/test_send_message_tool.py +++ b/tests/tools/test_send_message_tool.py @@ -825,9 +825,13 @@ class TestParseTargetRefSlack: def test_dm_id_is_explicit(self): assert _parse_target_ref("slack", "D123ABCDEF")[2] is True - def test_user_id_is_explicit(self): - assert _parse_target_ref("slack", "U123ABCDEF")[2] is True - assert _parse_target_ref("slack", "W123ABCDEF")[2] is True + def test_user_id_is_not_explicit(self): + """Slack user IDs (U...) and workspace IDs (W...) are NOT explicit send + targets. chat.postMessage rejects them — a DM must be opened first via + conversations.open to obtain a D... conversation ID. + """ + assert _parse_target_ref("slack", "U123ABCDEF")[2] is False + assert _parse_target_ref("slack", "W123ABCDEF")[2] is False def test_whitespace_is_stripped(self): chat_id, _, is_explicit = _parse_target_ref("slack", " C0B0QV5434G ") diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index cbf7e042e1..738cf6ca6f 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -20,10 +20,13 @@ logger = logging.getLogger(__name__) _TELEGRAM_TOPIC_TARGET_RE = re.compile(r"^\s*(-?\d+)(?::(\d+))?\s*$") _FEISHU_TARGET_RE = re.compile(r"^\s*((?:oc|ou|on|chat|open)_[-A-Za-z0-9]+)(?::([-A-Za-z0-9_]+))?\s*$") -# Slack channel/user IDs: C (public), G (private/group), D (DM), U/W (user). -# Always uppercase alphanumeric, 9+ chars. Without this, Slack IDs fall through -# to channel-name resolution, which only matches by name and fails. -_SLACK_TARGET_RE = re.compile(r"^\s*([CGDUW][A-Z0-9]{8,})\s*$") +# Slack conversation IDs: C (public channel), G (private/group channel), D (DM). +# Must be uppercase alphanumeric, 9+ chars. User IDs (U...) and workspace IDs +# (W...) are NOT valid chat.postMessage channel values — posting to them fails +# because the API requires a conversation ID. To DM a user you must first call +# 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*$") _WEIXIN_TARGET_RE = re.compile(r"^\s*((?:wxid|gh|v\d+|wm|wb)_[A-Za-z0-9_-]+|[A-Za-z0-9._-]+@chatroom|filehelper)\s*$") # Discord snowflake IDs are numeric, same regex pattern as Telegram topic targets. _NUMERIC_TOPIC_RE = _TELEGRAM_TOPIC_TARGET_RE