fix(slack): keep cross-target sends out of origin threads
When Slack resolves a named target to another conversation, do not reuse the origin thread timestamp on the destination send, and keep reaction cleanup anchored to the source conversation. Made-with: Cursor
This commit is contained in:
@@ -120,8 +120,13 @@ class SlackChannel(BaseChannel):
|
||||
slack_meta = msg.metadata.get("slack", {}) if msg.metadata else {}
|
||||
thread_ts = slack_meta.get("thread_ts")
|
||||
channel_type = slack_meta.get("channel_type")
|
||||
origin_chat_id = str((slack_meta.get("event", {}) or {}).get("channel") or msg.chat_id)
|
||||
# Slack DMs don't use threads; channel/group replies may keep thread_ts.
|
||||
thread_ts_param = thread_ts if thread_ts and channel_type != "im" else None
|
||||
thread_ts_param = (
|
||||
thread_ts
|
||||
if thread_ts and channel_type != "im" and target_chat_id == origin_chat_id
|
||||
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.
|
||||
@@ -145,7 +150,7 @@ class SlackChannel(BaseChannel):
|
||||
# Update reaction emoji when the final (non-progress) response is sent
|
||||
if not (msg.metadata or {}).get("_progress"):
|
||||
event = slack_meta.get("event", {})
|
||||
await self._update_react_emoji(event.get("channel") or msg.chat_id, event.get("ts"))
|
||||
await self._update_react_emoji(origin_chat_id, event.get("ts"))
|
||||
|
||||
except Exception as e:
|
||||
logger.error("Error sending Slack message: {}", e)
|
||||
|
||||
@@ -270,6 +270,38 @@ async def test_send_updates_reaction_on_origin_channel_for_cross_channel_send()
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_does_not_reuse_origin_thread_ts_for_cross_channel_send() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True), MessageBus())
|
||||
fake_web = _FakeAsyncWebClient()
|
||||
fake_web._conversations_pages = [
|
||||
{
|
||||
"channels": [{"id": "C999", "name": "channel_x"}],
|
||||
"response_metadata": {"next_cursor": ""},
|
||||
}
|
||||
]
|
||||
channel._web_client = fake_web
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="slack",
|
||||
chat_id="channel_x",
|
||||
content="done",
|
||||
metadata={
|
||||
"slack": {
|
||||
"event": {"ts": "1700000000.000100", "channel": "C_ORIGIN"},
|
||||
"thread_ts": "1700000000.000200",
|
||||
"channel_type": "channel",
|
||||
},
|
||||
},
|
||||
)
|
||||
)
|
||||
|
||||
assert fake_web.chat_post_calls == [
|
||||
{"channel": "C999", "text": "done\n", "thread_ts": None}
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_raises_when_named_target_cannot_be_resolved() -> None:
|
||||
channel = SlackChannel(SlackConfig(enabled=True), MessageBus())
|
||||
|
||||
Reference in New Issue
Block a user