mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-05-01 15:25:56 +00:00
fix(slack): polish thread UX and media support
This commit is contained in:
parent
5e9b9b9818
commit
8a0917db7a
@ -190,6 +190,7 @@ nanobot agent
|
||||
|
||||
- Want different LLM providers, web search, MCP, security settings, or more config options? See [Configuration](./docs/configuration.md)
|
||||
- Want to run nanobot in chat apps like Telegram, Discord, WeChat or Feishu? See [Chat Apps](./docs/chat-apps.md)
|
||||
- Using Slack? Add `files:write` if you want nanobot to upload images, videos, or files.
|
||||
- Want Docker or Linux service deployment? See [Deployment](./docs/deployment.md)
|
||||
|
||||
## 🧪 WebUI (Development)
|
||||
|
||||
@ -434,11 +434,13 @@ Uses **Socket Mode** — no public URL required.
|
||||
|
||||
**2. Configure the app**
|
||||
- **Socket Mode**: Toggle ON → Generate an **App-Level Token** with `connections:write` scope → copy it (`xapp-...`)
|
||||
- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `channels:history`, `groups:history`, `im:history`, `mpim:history`
|
||||
- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `files:write`, `channels:history`, `groups:history`, `im:history`, `mpim:history`
|
||||
- **Event Subscriptions**: Toggle ON → Subscribe to bot events: `message.im`, `message.channels`, `app_mention` → Save Changes
|
||||
- **App Home**: Scroll to **Show Tabs** → Enable **Messages Tab** → Check **"Allow users to send Slash commands and messages from the messages tab"**
|
||||
- **Install App**: Click **Install to Workspace** → Authorize → copy the **Bot Token** (`xoxb-...`)
|
||||
|
||||
> `files:write` is required for images, videos, and other file uploads. If you add it later, reinstall the Slack app to the workspace and restart nanobot so it uses the updated bot token.
|
||||
|
||||
**3. Configure nanobot**
|
||||
|
||||
```json
|
||||
|
||||
@ -16,6 +16,7 @@ from nanobot.bus.events import OutboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.base import BaseChannel
|
||||
from nanobot.config.schema import Base
|
||||
from nanobot.utils.helpers import split_message
|
||||
|
||||
|
||||
class SlackDMConfig(Base):
|
||||
@ -46,6 +47,9 @@ class SlackConfig(Base):
|
||||
dm: SlackDMConfig = Field(default_factory=SlackDMConfig)
|
||||
|
||||
|
||||
SLACK_MAX_MESSAGE_LEN = 39_000 # Slack API allows ~40k; leave margin
|
||||
|
||||
|
||||
class SlackChannel(BaseChannel):
|
||||
"""Slack channel using Socket Mode."""
|
||||
|
||||
@ -59,6 +63,8 @@ class SlackChannel(BaseChannel):
|
||||
def default_config(cls) -> dict[str, Any]:
|
||||
return SlackConfig().model_dump(by_alias=True)
|
||||
|
||||
_THREAD_CONTEXT_CACHE_LIMIT = 10_000
|
||||
|
||||
def __init__(self, config: Any, bus: MessageBus):
|
||||
if isinstance(config, dict):
|
||||
config = SlackConfig.model_validate(config)
|
||||
@ -131,14 +137,17 @@ class SlackChannel(BaseChannel):
|
||||
else None
|
||||
)
|
||||
|
||||
# Slack rejects empty text payloads. Keep media-only messages media-only,
|
||||
# but send a single blank message when the bot has no text or files to send.
|
||||
if msg.content or not (msg.media or []):
|
||||
await self._web_client.chat_postMessage(
|
||||
channel=target_chat_id,
|
||||
text=self._to_mrkdwn(msg.content) if msg.content else " ",
|
||||
thread_ts=thread_ts_param,
|
||||
)
|
||||
mrkdwn = self._to_mrkdwn(msg.content) if msg.content else " "
|
||||
buttons = getattr(msg, "buttons", None) or []
|
||||
chunks = split_message(mrkdwn, SLACK_MAX_MESSAGE_LEN)
|
||||
for index, chunk in enumerate(chunks):
|
||||
kwargs: dict[str, Any] = dict(
|
||||
channel=target_chat_id, text=chunk, thread_ts=thread_ts_param,
|
||||
)
|
||||
if buttons and index == len(chunks) - 1:
|
||||
kwargs["blocks"] = self._build_button_blocks(chunk, buttons)
|
||||
await self._web_client.chat_postMessage(**kwargs)
|
||||
|
||||
for media_path in msg.media or []:
|
||||
try:
|
||||
@ -276,6 +285,9 @@ class SlackChannel(BaseChannel):
|
||||
req: SocketModeRequest,
|
||||
) -> None:
|
||||
"""Handle incoming Socket Mode requests."""
|
||||
if req.type == "interactive":
|
||||
await self._on_block_action(client, req)
|
||||
return
|
||||
if req.type != "events_api":
|
||||
return
|
||||
|
||||
@ -375,6 +387,37 @@ class SlackChannel(BaseChannel):
|
||||
except Exception:
|
||||
logger.exception("Error handling Slack message from {}", sender_id)
|
||||
|
||||
async def _on_block_action(self, client: SocketModeClient, req: SocketModeRequest) -> None:
|
||||
"""Handle button clicks from ask_user blocks."""
|
||||
await client.send_socket_mode_response(SocketModeResponse(envelope_id=req.envelope_id))
|
||||
payload = req.payload or {}
|
||||
actions = payload.get("actions") or []
|
||||
if not actions:
|
||||
return
|
||||
value = str(actions[0].get("value") or "")
|
||||
user_info = payload.get("user") or {}
|
||||
sender_id = str(user_info.get("id") or "")
|
||||
channel_info = payload.get("channel") or {}
|
||||
chat_id = str(channel_info.get("id") or "")
|
||||
if not sender_id or not chat_id or not value:
|
||||
return
|
||||
message_info = payload.get("message") or {}
|
||||
thread_ts = message_info.get("thread_ts") or message_info.get("ts")
|
||||
channel_type = self._infer_channel_type(chat_id)
|
||||
if not self._is_allowed(sender_id, chat_id, channel_type):
|
||||
return
|
||||
session_key = f"slack:{chat_id}:{thread_ts}" if thread_ts else None
|
||||
try:
|
||||
await self._handle_message(
|
||||
sender_id=sender_id,
|
||||
chat_id=chat_id,
|
||||
content=value,
|
||||
metadata={"slack": {"thread_ts": thread_ts, "channel_type": channel_type}},
|
||||
session_key=session_key,
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Error handling Slack button click from {}", sender_id)
|
||||
|
||||
async def _with_thread_context(
|
||||
self,
|
||||
text: str,
|
||||
@ -399,6 +442,8 @@ class SlackChannel(BaseChannel):
|
||||
key = f"{chat_id}:{thread_ts}"
|
||||
if key in self._thread_context_attempted:
|
||||
return text
|
||||
if len(self._thread_context_attempted) >= self._THREAD_CONTEXT_CACHE_LIMIT:
|
||||
self._thread_context_attempted.clear()
|
||||
self._thread_context_attempted.add(key)
|
||||
|
||||
try:
|
||||
@ -427,14 +472,36 @@ class SlackChannel(BaseChannel):
|
||||
if item.get("subtype"):
|
||||
continue
|
||||
sender = str(item.get("user") or item.get("bot_id") or "unknown")
|
||||
if self._bot_user_id and sender == self._bot_user_id:
|
||||
continue
|
||||
is_bot = self._bot_user_id is not None and sender == self._bot_user_id
|
||||
label = "bot" if is_bot else f"<@{sender}>"
|
||||
text = str(item.get("text") or "").strip()
|
||||
if not text:
|
||||
continue
|
||||
lines.append(f"- <@{sender}>: {self._strip_bot_mention(text)}")
|
||||
text = self._strip_bot_mention(text)
|
||||
if len(text) > 500:
|
||||
text = text[:500] + "…"
|
||||
lines.append(f"- {label}: {text}")
|
||||
return lines
|
||||
|
||||
@staticmethod
|
||||
def _build_button_blocks(text: str, buttons: list[list[str]]) -> list[dict[str, Any]]:
|
||||
"""Build Slack Block Kit blocks with action buttons for ask_user choices."""
|
||||
blocks: list[dict[str, Any]] = [
|
||||
{"type": "section", "text": {"type": "mrkdwn", "text": text[:3000]}},
|
||||
]
|
||||
elements = []
|
||||
for row in buttons:
|
||||
for label in row:
|
||||
elements.append({
|
||||
"type": "button",
|
||||
"text": {"type": "plain_text", "text": label[:75]},
|
||||
"value": label[:75],
|
||||
"action_id": f"ask_user_{label[:50]}",
|
||||
})
|
||||
if elements:
|
||||
blocks.append({"type": "actions", "elements": elements[:25]})
|
||||
return blocks
|
||||
|
||||
async def _update_react_emoji(self, chat_id: str, ts: str | None) -> None:
|
||||
"""Remove the in-progress reaction and optionally add a done reaction."""
|
||||
if not self._web_client or not ts:
|
||||
@ -481,6 +548,19 @@ class SlackChannel(BaseChannel):
|
||||
return chat_id in self.config.group_allow_from
|
||||
return False
|
||||
|
||||
def is_allowed(self, sender_id: str) -> bool:
|
||||
# Slack needs channel-aware policy checks, so _on_socket_request and
|
||||
# _on_block_action call _is_allowed before handing off to BaseChannel.
|
||||
return True
|
||||
|
||||
@staticmethod
|
||||
def _infer_channel_type(chat_id: str) -> str:
|
||||
if chat_id.startswith("D"):
|
||||
return "im"
|
||||
if chat_id.startswith("G"):
|
||||
return "group"
|
||||
return "channel"
|
||||
|
||||
def _strip_bot_mention(self, text: str) -> str:
|
||||
if not text or not self._bot_user_id:
|
||||
return text
|
||||
|
||||
@ -1,5 +1,8 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
# Check optional Slack dependencies before running tests
|
||||
@ -10,7 +13,7 @@ except ImportError:
|
||||
|
||||
from nanobot.bus.events import OutboundMessage
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.slack import SlackChannel, SlackConfig
|
||||
from nanobot.channels.slack import SLACK_MAX_MESSAGE_LEN, SlackChannel, SlackConfig
|
||||
|
||||
|
||||
class _FakeAsyncWebClient:
|
||||
@ -34,14 +37,16 @@ class _FakeAsyncWebClient:
|
||||
channel: str,
|
||||
text: str,
|
||||
thread_ts: str | None = None,
|
||||
blocks: list[dict[str, object]] | None = None,
|
||||
) -> None:
|
||||
self.chat_post_calls.append(
|
||||
{
|
||||
"channel": channel,
|
||||
"text": text,
|
||||
"thread_ts": thread_ts,
|
||||
}
|
||||
)
|
||||
call: dict[str, object | None] = {
|
||||
"channel": channel,
|
||||
"text": text,
|
||||
"thread_ts": thread_ts,
|
||||
}
|
||||
if blocks is not None:
|
||||
call["blocks"] = blocks
|
||||
self.chat_post_calls.append(call)
|
||||
|
||||
async def files_upload_v2(
|
||||
self,
|
||||
@ -155,6 +160,61 @@ async def test_send_omits_thread_for_dm_messages() -> None:
|
||||
assert fake_web.file_upload_calls[0]["thread_ts"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_splits_long_messages() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True), MessageBus())
|
||||
fake_web = _FakeAsyncWebClient()
|
||||
channel._web_client = fake_web
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="slack",
|
||||
chat_id="C123",
|
||||
content="x" * (SLACK_MAX_MESSAGE_LEN + 10),
|
||||
)
|
||||
)
|
||||
|
||||
assert len(fake_web.chat_post_calls) == 2
|
||||
assert all(len(str(call["text"])) <= SLACK_MAX_MESSAGE_LEN for call in fake_web.chat_post_calls)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_renders_buttons_on_last_message_chunk() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True), MessageBus())
|
||||
fake_web = _FakeAsyncWebClient()
|
||||
channel._web_client = fake_web
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="slack",
|
||||
chat_id="C123",
|
||||
content="Choose one",
|
||||
buttons=[["Yes", "No"]],
|
||||
)
|
||||
)
|
||||
|
||||
assert len(fake_web.chat_post_calls) == 1
|
||||
blocks = fake_web.chat_post_calls[0]["blocks"]
|
||||
assert isinstance(blocks, list)
|
||||
assert blocks[-1] == {
|
||||
"type": "actions",
|
||||
"elements": [
|
||||
{
|
||||
"type": "button",
|
||||
"text": {"type": "plain_text", "text": "Yes"},
|
||||
"value": "Yes",
|
||||
"action_id": "ask_user_Yes",
|
||||
},
|
||||
{
|
||||
"type": "button",
|
||||
"text": {"type": "plain_text", "text": "No"},
|
||||
"value": "No",
|
||||
"action_id": "ask_user_No",
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_updates_reaction_when_final_response_sent() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True, react_emoji="eyes"), MessageBus())
|
||||
@ -333,6 +393,7 @@ async def test_with_thread_context_fetches_root_once() -> None:
|
||||
"messages": [
|
||||
{"ts": "111.000", "user": "UROOT", "text": "drink water"},
|
||||
{"ts": "112.000", "user": "U2", "text": "good idea"},
|
||||
{"ts": "112.500", "user": "UBOT", "text": "I'll remind you."},
|
||||
{"ts": "113.000", "user": "U3", "text": "<@UBOT> what did you see?"},
|
||||
]
|
||||
}
|
||||
@ -353,6 +414,7 @@ async def test_with_thread_context_fetches_root_once() -> None:
|
||||
assert "Slack thread context before this mention:" in content
|
||||
assert "- <@UROOT>: drink water" in content
|
||||
assert "- <@U2>: good idea" in content
|
||||
assert "- bot: I'll remind you." in content
|
||||
assert "U3" not in content
|
||||
assert content.endswith("Current message:\nwhat did you see?")
|
||||
|
||||
@ -366,3 +428,38 @@ async def test_with_thread_context_fetches_root_once() -> None:
|
||||
)
|
||||
assert second == "again"
|
||||
assert len(fake_web.conversations_replies_calls) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_slack_slash_command_skips_thread_context() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus())
|
||||
channel._bot_user_id = "UBOT"
|
||||
channel._with_thread_context = AsyncMock(return_value="wrapped") # type: ignore[method-assign]
|
||||
channel._handle_message = AsyncMock() # type: ignore[method-assign]
|
||||
client = SimpleNamespace(send_socket_mode_response=AsyncMock())
|
||||
req = SimpleNamespace(
|
||||
type="events_api",
|
||||
envelope_id="env-1",
|
||||
payload={
|
||||
"event": {
|
||||
"type": "app_mention",
|
||||
"user": "U1",
|
||||
"channel": "C123",
|
||||
"text": "<@UBOT> /restart",
|
||||
"thread_ts": "111.000",
|
||||
"ts": "112.000",
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
await channel._on_socket_request(client, req)
|
||||
|
||||
channel._with_thread_context.assert_not_awaited()
|
||||
channel._handle_message.assert_awaited_once()
|
||||
assert channel._handle_message.await_args.kwargs["content"] == "/restart"
|
||||
|
||||
|
||||
def test_slack_channel_uses_channel_aware_allow_policy() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus())
|
||||
assert channel.is_allowed("U1") is True
|
||||
assert channel._is_allowed("U1", "C123", "channel") is True
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user