feat: Slack adapter improvements — formatting, reactions, user resolution, commands (#1106)
feat: Slack adapter improvements — formatting, reactions, user resolution, commands
This commit is contained in:
Teknium
@@ -66,13 +66,14 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
- Typing indicators (not natively supported by Slack bots)
|
||||
"""
|
||||
|
||||
MAX_MESSAGE_LENGTH = 4000 # Slack's limit is higher but mrkdwn can inflate
|
||||
MAX_MESSAGE_LENGTH = 3900 # Slack hard limit is 4000 but leave room for mrkdwn
|
||||
|
||||
def __init__(self, config: PlatformConfig):
|
||||
super().__init__(config, Platform.SLACK)
|
||||
self._app: Optional[AsyncApp] = None
|
||||
self._handler: Optional[AsyncSocketModeHandler] = None
|
||||
self._bot_user_id: Optional[str] = None
|
||||
self._user_name_cache: Dict[str, str] = {} # user_id → display name
|
||||
|
||||
async def connect(self) -> bool:
|
||||
"""Connect to Slack via Socket Mode."""
|
||||
@@ -152,22 +153,36 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
return SendResult(success=False, error="Not connected")
|
||||
|
||||
try:
|
||||
kwargs = {
|
||||
"channel": chat_id,
|
||||
"text": content,
|
||||
}
|
||||
# Convert standard markdown → Slack mrkdwn
|
||||
formatted = self.format_message(content)
|
||||
|
||||
# Split long messages, preserving code block boundaries
|
||||
chunks = self.truncate_message(formatted, self.MAX_MESSAGE_LENGTH)
|
||||
|
||||
# Reply in thread if thread context is available.
|
||||
thread_ts = self._resolve_thread_ts(reply_to, metadata)
|
||||
if thread_ts:
|
||||
kwargs["thread_ts"] = thread_ts
|
||||
last_result = None
|
||||
|
||||
result = await self._app.client.chat_postMessage(**kwargs)
|
||||
# reply_broadcast: also post thread replies to the main channel.
|
||||
# Controlled via platform config: gateway.slack.reply_broadcast
|
||||
broadcast = self.config.extra.get("reply_broadcast", False)
|
||||
|
||||
for i, chunk in enumerate(chunks):
|
||||
kwargs = {
|
||||
"channel": chat_id,
|
||||
"text": chunk,
|
||||
}
|
||||
if thread_ts:
|
||||
kwargs["thread_ts"] = thread_ts
|
||||
# Only broadcast the first chunk of the first reply
|
||||
if broadcast and i == 0:
|
||||
kwargs["reply_broadcast"] = True
|
||||
|
||||
last_result = await self._app.client.chat_postMessage(**kwargs)
|
||||
|
||||
return SendResult(
|
||||
success=True,
|
||||
message_id=result.get("ts"),
|
||||
raw_response=result,
|
||||
message_id=last_result.get("ts") if last_result else None,
|
||||
raw_response=last_result,
|
||||
)
|
||||
|
||||
except Exception as e: # pragma: no cover - defensive logging
|
||||
@@ -201,7 +216,11 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
return SendResult(success=False, error=str(e))
|
||||
|
||||
async def send_typing(self, chat_id: str, metadata=None) -> None:
|
||||
"""Slack doesn't have a direct typing indicator API for bots."""
|
||||
"""Slack doesn't support typing indicators for bot users.
|
||||
|
||||
The reactions system (👀 on receipt, ✅ on completion) serves as
|
||||
the visual feedback mechanism instead.
|
||||
"""
|
||||
pass
|
||||
|
||||
def _resolve_thread_ts(
|
||||
@@ -221,6 +240,154 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
return metadata["thread_ts"]
|
||||
return reply_to
|
||||
|
||||
# ----- Markdown → mrkdwn conversion -----
|
||||
|
||||
def format_message(self, content: str) -> str:
|
||||
"""Convert standard markdown to Slack mrkdwn format.
|
||||
|
||||
Protected regions (code blocks, inline code) are extracted first so
|
||||
their contents are never modified. Standard markdown constructs
|
||||
(headers, bold, italic, links) are translated to mrkdwn syntax.
|
||||
"""
|
||||
if not content:
|
||||
return content
|
||||
|
||||
placeholders: dict = {}
|
||||
counter = [0]
|
||||
|
||||
def _ph(value: str) -> str:
|
||||
"""Stash value behind a placeholder that survives later passes."""
|
||||
key = f"\x00SL{counter[0]}\x00"
|
||||
counter[0] += 1
|
||||
placeholders[key] = value
|
||||
return key
|
||||
|
||||
text = content
|
||||
|
||||
# 1) Protect fenced code blocks (``` ... ```)
|
||||
text = re.sub(
|
||||
r'(```(?:[^\n]*\n)?[\s\S]*?```)',
|
||||
lambda m: _ph(m.group(0)),
|
||||
text,
|
||||
)
|
||||
|
||||
# 2) Protect inline code (`...`)
|
||||
text = re.sub(r'(`[^`]+`)', lambda m: _ph(m.group(0)), text)
|
||||
|
||||
# 3) Convert markdown links [text](url) → <url|text>
|
||||
text = re.sub(
|
||||
r'\[([^\]]+)\]\(([^)]+)\)',
|
||||
lambda m: _ph(f'<{m.group(2)}|{m.group(1)}>'),
|
||||
text,
|
||||
)
|
||||
|
||||
# 4) Convert headers (## Title) → *Title* (bold)
|
||||
def _convert_header(m):
|
||||
inner = m.group(1).strip()
|
||||
# Strip redundant bold markers inside a header
|
||||
inner = re.sub(r'\*\*(.+?)\*\*', r'\1', inner)
|
||||
return _ph(f'*{inner}*')
|
||||
|
||||
text = re.sub(
|
||||
r'^#{1,6}\s+(.+)$', _convert_header, text, flags=re.MULTILINE
|
||||
)
|
||||
|
||||
# 5) Convert bold: **text** → *text* (Slack bold)
|
||||
text = re.sub(
|
||||
r'\*\*(.+?)\*\*',
|
||||
lambda m: _ph(f'*{m.group(1)}*'),
|
||||
text,
|
||||
)
|
||||
|
||||
# 6) Convert italic: _text_ stays as _text_ (already Slack italic)
|
||||
# Single *text* → _text_ (Slack italic)
|
||||
text = re.sub(
|
||||
r'(?<!\*)\*([^*\n]+)\*(?!\*)',
|
||||
lambda m: _ph(f'_{m.group(1)}_'),
|
||||
text,
|
||||
)
|
||||
|
||||
# 7) Convert strikethrough: ~~text~~ → ~text~
|
||||
text = re.sub(
|
||||
r'~~(.+?)~~',
|
||||
lambda m: _ph(f'~{m.group(1)}~'),
|
||||
text,
|
||||
)
|
||||
|
||||
# 8) Convert blockquotes: > text → > text (same syntax, just ensure
|
||||
# no extra escaping happens to the > character)
|
||||
# Slack uses the same > prefix, so this is a no-op for content.
|
||||
|
||||
# 9) Restore placeholders in reverse order
|
||||
for key in reversed(list(placeholders.keys())):
|
||||
text = text.replace(key, placeholders[key])
|
||||
|
||||
return text
|
||||
|
||||
# ----- Reactions -----
|
||||
|
||||
async def _add_reaction(
|
||||
self, channel: str, timestamp: str, emoji: str
|
||||
) -> bool:
|
||||
"""Add an emoji reaction to a message. Returns True on success."""
|
||||
if not self._app:
|
||||
return False
|
||||
try:
|
||||
await self._app.client.reactions_add(
|
||||
channel=channel, timestamp=timestamp, name=emoji
|
||||
)
|
||||
return True
|
||||
except Exception as e:
|
||||
# Don't log as error — may fail if already reacted or missing scope
|
||||
logger.debug("[Slack] reactions.add failed (%s): %s", emoji, e)
|
||||
return False
|
||||
|
||||
async def _remove_reaction(
|
||||
self, channel: str, timestamp: str, emoji: str
|
||||
) -> bool:
|
||||
"""Remove an emoji reaction from a message. Returns True on success."""
|
||||
if not self._app:
|
||||
return False
|
||||
try:
|
||||
await self._app.client.reactions_remove(
|
||||
channel=channel, timestamp=timestamp, name=emoji
|
||||
)
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.debug("[Slack] reactions.remove failed (%s): %s", emoji, e)
|
||||
return False
|
||||
|
||||
# ----- User identity resolution -----
|
||||
|
||||
async def _resolve_user_name(self, user_id: str) -> str:
|
||||
"""Resolve a Slack user ID to a display name, with caching."""
|
||||
if not user_id:
|
||||
return ""
|
||||
if user_id in self._user_name_cache:
|
||||
return self._user_name_cache[user_id]
|
||||
|
||||
if not self._app:
|
||||
return user_id
|
||||
|
||||
try:
|
||||
result = await self._app.client.users_info(user=user_id)
|
||||
user = result.get("user", {})
|
||||
# Prefer display_name → real_name → user_id
|
||||
profile = user.get("profile", {})
|
||||
name = (
|
||||
profile.get("display_name")
|
||||
or profile.get("real_name")
|
||||
or user.get("real_name")
|
||||
or user.get("name")
|
||||
or user_id
|
||||
)
|
||||
self._user_name_cache[user_id] = name
|
||||
return name
|
||||
except Exception as e:
|
||||
logger.debug("[Slack] users.info failed for %s: %s", user_id, e)
|
||||
self._user_name_cache[user_id] = user_id
|
||||
return user_id
|
||||
|
||||
async def send_image_file(
|
||||
self,
|
||||
chat_id: str,
|
||||
@@ -551,12 +718,16 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
except Exception as e: # pragma: no cover - defensive logging
|
||||
logger.warning("[Slack] Failed to cache document from %s: %s", url, e, exc_info=True)
|
||||
|
||||
# Resolve user display name (cached after first lookup)
|
||||
user_name = await self._resolve_user_name(user_id)
|
||||
|
||||
# Build source
|
||||
source = self.build_source(
|
||||
chat_id=channel_id,
|
||||
chat_name=channel_id, # Will be resolved later if needed
|
||||
chat_type="dm" if is_dm else "group",
|
||||
user_id=user_id,
|
||||
user_name=user_name,
|
||||
thread_id=thread_ts,
|
||||
)
|
||||
|
||||
@@ -571,8 +742,15 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
reply_to_message_id=thread_ts if thread_ts != ts else None,
|
||||
)
|
||||
|
||||
# Add 👀 reaction to acknowledge receipt
|
||||
await self._add_reaction(channel_id, ts, "eyes")
|
||||
|
||||
await self.handle_message(msg_event)
|
||||
|
||||
# Replace 👀 with ✅ when done
|
||||
await self._remove_reaction(channel_id, ts, "eyes")
|
||||
await self._add_reaction(channel_id, ts, "white_check_mark")
|
||||
|
||||
async def _handle_slash_command(self, command: dict) -> None:
|
||||
"""Handle /hermes slash command."""
|
||||
text = command.get("text", "").strip()
|
||||
@@ -586,6 +764,15 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
"help": "/help",
|
||||
"model": "/model", "personality": "/personality",
|
||||
"retry": "/retry", "undo": "/undo",
|
||||
"compact": "/compress", "compress": "/compress",
|
||||
"resume": "/resume",
|
||||
"background": "/background",
|
||||
"usage": "/usage",
|
||||
"insights": "/insights",
|
||||
"title": "/title",
|
||||
"reasoning": "/reasoning",
|
||||
"provider": "/provider",
|
||||
"rollback": "/rollback",
|
||||
}
|
||||
first_word = text.split()[0] if text else ""
|
||||
if first_word in subcommand_map:
|
||||
|
||||
@@ -530,3 +530,277 @@ class TestMessageRouting:
|
||||
}
|
||||
await adapter._handle_slack_message(event)
|
||||
adapter.handle_message.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestFormatMessage — Markdown → mrkdwn conversion
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFormatMessage:
|
||||
"""Test markdown to Slack mrkdwn conversion."""
|
||||
|
||||
def test_bold_conversion(self, adapter):
|
||||
assert adapter.format_message("**hello**") == "*hello*"
|
||||
|
||||
def test_italic_asterisk_conversion(self, adapter):
|
||||
assert adapter.format_message("*hello*") == "_hello_"
|
||||
|
||||
def test_italic_underscore_preserved(self, adapter):
|
||||
assert adapter.format_message("_hello_") == "_hello_"
|
||||
|
||||
def test_header_to_bold(self, adapter):
|
||||
assert adapter.format_message("## Section Title") == "*Section Title*"
|
||||
|
||||
def test_header_with_bold_content(self, adapter):
|
||||
# **bold** inside a header should not double-wrap
|
||||
assert adapter.format_message("## **Title**") == "*Title*"
|
||||
|
||||
def test_link_conversion(self, adapter):
|
||||
result = adapter.format_message("[click here](https://example.com)")
|
||||
assert result == "<https://example.com|click here>"
|
||||
|
||||
def test_strikethrough(self, adapter):
|
||||
assert adapter.format_message("~~deleted~~") == "~deleted~"
|
||||
|
||||
def test_code_block_preserved(self, adapter):
|
||||
code = "```python\nx = **not bold**\n```"
|
||||
assert adapter.format_message(code) == code
|
||||
|
||||
def test_inline_code_preserved(self, adapter):
|
||||
text = "Use `**raw**` syntax"
|
||||
assert adapter.format_message(text) == "Use `**raw**` syntax"
|
||||
|
||||
def test_mixed_content(self, adapter):
|
||||
text = "**Bold** and *italic* with `code`"
|
||||
result = adapter.format_message(text)
|
||||
assert "*Bold*" in result
|
||||
assert "_italic_" in result
|
||||
assert "`code`" in result
|
||||
|
||||
def test_empty_string(self, adapter):
|
||||
assert adapter.format_message("") == ""
|
||||
|
||||
def test_none_passthrough(self, adapter):
|
||||
assert adapter.format_message(None) is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestReactions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestReactions:
|
||||
"""Test emoji reaction methods."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_reaction_calls_api(self, adapter):
|
||||
adapter._app.client.reactions_add = AsyncMock()
|
||||
result = await adapter._add_reaction("C123", "ts1", "eyes")
|
||||
assert result is True
|
||||
adapter._app.client.reactions_add.assert_called_once_with(
|
||||
channel="C123", timestamp="ts1", name="eyes"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_reaction_handles_error(self, adapter):
|
||||
adapter._app.client.reactions_add = AsyncMock(side_effect=Exception("already_reacted"))
|
||||
result = await adapter._add_reaction("C123", "ts1", "eyes")
|
||||
assert result is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_remove_reaction_calls_api(self, adapter):
|
||||
adapter._app.client.reactions_remove = AsyncMock()
|
||||
result = await adapter._remove_reaction("C123", "ts1", "eyes")
|
||||
assert result is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reactions_in_message_flow(self, adapter):
|
||||
"""Reactions should be added on receipt and swapped on completion."""
|
||||
adapter._app.client.reactions_add = AsyncMock()
|
||||
adapter._app.client.reactions_remove = AsyncMock()
|
||||
adapter._app.client.users_info = AsyncMock(return_value={
|
||||
"user": {"profile": {"display_name": "Tyler"}}
|
||||
})
|
||||
|
||||
event = {
|
||||
"text": "hello",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"channel_type": "im",
|
||||
"ts": "1234567890.000001",
|
||||
}
|
||||
await adapter._handle_slack_message(event)
|
||||
|
||||
# Should have added 👀, then removed 👀, then added ✅
|
||||
add_calls = adapter._app.client.reactions_add.call_args_list
|
||||
remove_calls = adapter._app.client.reactions_remove.call_args_list
|
||||
assert len(add_calls) == 2
|
||||
assert add_calls[0].kwargs["name"] == "eyes"
|
||||
assert add_calls[1].kwargs["name"] == "white_check_mark"
|
||||
assert len(remove_calls) == 1
|
||||
assert remove_calls[0].kwargs["name"] == "eyes"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestUserNameResolution
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestUserNameResolution:
|
||||
"""Test user identity resolution."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolves_display_name(self, adapter):
|
||||
adapter._app.client.users_info = AsyncMock(return_value={
|
||||
"user": {"profile": {"display_name": "Tyler", "real_name": "Tyler B"}}
|
||||
})
|
||||
name = await adapter._resolve_user_name("U123")
|
||||
assert name == "Tyler"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_falls_back_to_real_name(self, adapter):
|
||||
adapter._app.client.users_info = AsyncMock(return_value={
|
||||
"user": {"profile": {"display_name": "", "real_name": "Tyler B"}}
|
||||
})
|
||||
name = await adapter._resolve_user_name("U123")
|
||||
assert name == "Tyler B"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_caches_result(self, adapter):
|
||||
adapter._app.client.users_info = AsyncMock(return_value={
|
||||
"user": {"profile": {"display_name": "Tyler"}}
|
||||
})
|
||||
await adapter._resolve_user_name("U123")
|
||||
await adapter._resolve_user_name("U123")
|
||||
# Only one API call despite two lookups
|
||||
assert adapter._app.client.users_info.call_count == 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handles_api_error(self, adapter):
|
||||
adapter._app.client.users_info = AsyncMock(side_effect=Exception("rate limited"))
|
||||
name = await adapter._resolve_user_name("U123")
|
||||
assert name == "U123" # Falls back to user_id
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_name_in_message_source(self, adapter):
|
||||
"""Message source should include resolved user name."""
|
||||
adapter._app.client.users_info = AsyncMock(return_value={
|
||||
"user": {"profile": {"display_name": "Tyler"}}
|
||||
})
|
||||
adapter._app.client.reactions_add = AsyncMock()
|
||||
adapter._app.client.reactions_remove = AsyncMock()
|
||||
|
||||
event = {
|
||||
"text": "hello",
|
||||
"user": "U_USER",
|
||||
"channel": "C123",
|
||||
"channel_type": "im",
|
||||
"ts": "1234567890.000001",
|
||||
}
|
||||
await adapter._handle_slack_message(event)
|
||||
|
||||
# Check the source in the MessageEvent passed to handle_message
|
||||
msg_event = adapter.handle_message.call_args[0][0]
|
||||
assert msg_event.source.user_name == "Tyler"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestSlashCommands — expanded command set
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSlashCommands:
|
||||
"""Test slash command routing."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_compact_maps_to_compress(self, adapter):
|
||||
command = {"text": "compact", "user_id": "U1", "channel_id": "C1"}
|
||||
await adapter._handle_slash_command(command)
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == "/compress"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resume_command(self, adapter):
|
||||
command = {"text": "resume my session", "user_id": "U1", "channel_id": "C1"}
|
||||
await adapter._handle_slash_command(command)
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == "/resume my session"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_background_command(self, adapter):
|
||||
command = {"text": "background run tests", "user_id": "U1", "channel_id": "C1"}
|
||||
await adapter._handle_slash_command(command)
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == "/background run tests"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_usage_command(self, adapter):
|
||||
command = {"text": "usage", "user_id": "U1", "channel_id": "C1"}
|
||||
await adapter._handle_slash_command(command)
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == "/usage"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reasoning_command(self, adapter):
|
||||
command = {"text": "reasoning", "user_id": "U1", "channel_id": "C1"}
|
||||
await adapter._handle_slash_command(command)
|
||||
msg = adapter.handle_message.call_args[0][0]
|
||||
assert msg.text == "/reasoning"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestMessageSplitting
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestMessageSplitting:
|
||||
"""Test that long messages are split before sending."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_long_message_split_into_chunks(self, adapter):
|
||||
"""Messages over MAX_MESSAGE_LENGTH should be split."""
|
||||
long_text = "x" * 5000
|
||||
adapter._app.client.chat_postMessage = AsyncMock(
|
||||
return_value={"ts": "ts1"}
|
||||
)
|
||||
await adapter.send("C123", long_text)
|
||||
# Should have been called multiple times
|
||||
assert adapter._app.client.chat_postMessage.call_count >= 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_short_message_single_send(self, adapter):
|
||||
"""Short messages should be sent in one call."""
|
||||
adapter._app.client.chat_postMessage = AsyncMock(
|
||||
return_value={"ts": "ts1"}
|
||||
)
|
||||
await adapter.send("C123", "hello world")
|
||||
assert adapter._app.client.chat_postMessage.call_count == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestReplyBroadcast
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestReplyBroadcast:
|
||||
"""Test reply_broadcast config option."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_broadcast_disabled_by_default(self, adapter):
|
||||
adapter._app.client.chat_postMessage = AsyncMock(
|
||||
return_value={"ts": "ts1"}
|
||||
)
|
||||
await adapter.send("C123", "hi", metadata={"thread_id": "parent_ts"})
|
||||
kwargs = adapter._app.client.chat_postMessage.call_args.kwargs
|
||||
assert "reply_broadcast" not in kwargs
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_broadcast_enabled_via_config(self, adapter):
|
||||
adapter.config.extra["reply_broadcast"] = True
|
||||
adapter._app.client.chat_postMessage = AsyncMock(
|
||||
return_value={"ts": "ts1"}
|
||||
)
|
||||
await adapter.send("C123", "hi", metadata={"thread_id": "parent_ts"})
|
||||
kwargs = adapter._app.client.chat_postMessage.call_args.kwargs
|
||||
assert kwargs.get("reply_broadcast") is True
|
||||
|
||||
Reference in New Issue
Block a user