fix(slack): comprehensive mrkdwn formatting — 6 bug fixes + 52 tests
Fixes blockquote > escaping, edit_message raw markdown, ***bold italic*** handling, HTML entity double-escaping (&amp;), Wikipedia URL parens truncation, and step numbering format. Also adds format_message to the tool-layer _send_to_platform for consistent formatting across all delivery paths. Changes: - Protect Slack entities (<@user>, <https://...|label>, <!here>) from escaping passes - Protect blockquote > markers before HTML entity escaping - Unescape-before-escape for idempotent HTML entity handling - ***bold italic*** → *_text_* conversion (before **bold** pass) - URL regex upgraded to handle balanced parentheses - mrkdwn:True flag on chat_postMessage payloads - format_message applied in edit_message and send_message_tool - 52 new tests (format, edit, streaming, splitting, tool chunking) - Use reversed(dict) idiom for placeholder restoration Based on PR #3715 by dashed, cherry-picked onto current main.
This commit is contained in:
@@ -32,6 +32,30 @@ def _install_telegram_mock(monkeypatch, bot):
|
||||
monkeypatch.setitem(sys.modules, "telegram.constants", constants_mod)
|
||||
|
||||
|
||||
def _ensure_slack_mock(monkeypatch):
|
||||
if "slack_bolt" in sys.modules and hasattr(sys.modules["slack_bolt"], "__file__"):
|
||||
return
|
||||
|
||||
slack_bolt = MagicMock()
|
||||
slack_bolt.async_app.AsyncApp = MagicMock
|
||||
slack_bolt.adapter.socket_mode.async_handler.AsyncSocketModeHandler = MagicMock
|
||||
|
||||
slack_sdk = MagicMock()
|
||||
slack_sdk.web.async_client.AsyncWebClient = MagicMock
|
||||
|
||||
for name, mod in [
|
||||
("slack_bolt", slack_bolt),
|
||||
("slack_bolt.async_app", slack_bolt.async_app),
|
||||
("slack_bolt.adapter", slack_bolt.adapter),
|
||||
("slack_bolt.adapter.socket_mode", slack_bolt.adapter.socket_mode),
|
||||
("slack_bolt.adapter.socket_mode.async_handler", slack_bolt.adapter.socket_mode.async_handler),
|
||||
("slack_sdk", slack_sdk),
|
||||
("slack_sdk.web", slack_sdk.web),
|
||||
("slack_sdk.web.async_client", slack_sdk.web.async_client),
|
||||
]:
|
||||
monkeypatch.setitem(sys.modules, name, mod)
|
||||
|
||||
|
||||
class TestSendMessageTool:
|
||||
def test_cron_duplicate_target_is_skipped_and_explained(self):
|
||||
home = SimpleNamespace(chat_id="-1001")
|
||||
@@ -426,7 +450,7 @@ class TestSendToPlatformChunking:
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.DISCORD,
|
||||
SimpleNamespace(enabled=True, token="tok", extra={}),
|
||||
SimpleNamespace(enabled=True, token="***", extra={}),
|
||||
"ch", long_msg,
|
||||
)
|
||||
)
|
||||
@@ -435,8 +459,115 @@ class TestSendToPlatformChunking:
|
||||
for call in send.await_args_list:
|
||||
assert len(call.args[2]) <= 2020 # each chunk fits the limit
|
||||
|
||||
def test_slack_messages_are_formatted_before_send(self, monkeypatch):
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
|
||||
import gateway.platforms.slack as slack_mod
|
||||
|
||||
monkeypatch.setattr(slack_mod, "SLACK_AVAILABLE", True)
|
||||
send = AsyncMock(return_value={"success": True, "message_id": "1"})
|
||||
|
||||
with patch("tools.send_message_tool._send_slack", send):
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.SLACK,
|
||||
SimpleNamespace(enabled=True, token="***", extra={}),
|
||||
"C123",
|
||||
"**hello** from [Hermes](<https://example.com>)",
|
||||
)
|
||||
)
|
||||
|
||||
assert result["success"] is True
|
||||
send.assert_awaited_once_with(
|
||||
"***",
|
||||
"C123",
|
||||
"*hello* from <https://example.com|Hermes>",
|
||||
)
|
||||
|
||||
def test_slack_bold_italic_formatted_before_send(self, monkeypatch):
|
||||
"""Bold+italic ***text*** survives tool-layer formatting."""
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
import gateway.platforms.slack as slack_mod
|
||||
|
||||
monkeypatch.setattr(slack_mod, "SLACK_AVAILABLE", True)
|
||||
send = AsyncMock(return_value={"success": True, "message_id": "1"})
|
||||
with patch("tools.send_message_tool._send_slack", send):
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.SLACK,
|
||||
SimpleNamespace(enabled=True, token="***", extra={}),
|
||||
"C123",
|
||||
"***important*** update",
|
||||
)
|
||||
)
|
||||
assert result["success"] is True
|
||||
sent_text = send.await_args.args[2]
|
||||
assert "*_important_*" in sent_text
|
||||
|
||||
def test_slack_blockquote_formatted_before_send(self, monkeypatch):
|
||||
"""Blockquote '>' markers must survive formatting (not escaped to '>')."""
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
import gateway.platforms.slack as slack_mod
|
||||
|
||||
monkeypatch.setattr(slack_mod, "SLACK_AVAILABLE", True)
|
||||
send = AsyncMock(return_value={"success": True, "message_id": "1"})
|
||||
with patch("tools.send_message_tool._send_slack", send):
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.SLACK,
|
||||
SimpleNamespace(enabled=True, token="***", extra={}),
|
||||
"C123",
|
||||
"> important quote\n\nnormal text & stuff",
|
||||
)
|
||||
)
|
||||
assert result["success"] is True
|
||||
sent_text = send.await_args.args[2]
|
||||
assert sent_text.startswith("> important quote")
|
||||
assert "&" in sent_text # & is escaped
|
||||
assert ">" not in sent_text.split("\n")[0] # > in blockquote is NOT escaped
|
||||
|
||||
def test_slack_pre_escaped_entities_not_double_escaped(self, monkeypatch):
|
||||
"""Pre-escaped HTML entities survive tool-layer formatting without double-escaping."""
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
import gateway.platforms.slack as slack_mod
|
||||
monkeypatch.setattr(slack_mod, "SLACK_AVAILABLE", True)
|
||||
send = AsyncMock(return_value={"success": True, "message_id": "1"})
|
||||
with patch("tools.send_message_tool._send_slack", send):
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.SLACK,
|
||||
SimpleNamespace(enabled=True, token="***", extra={}),
|
||||
"C123",
|
||||
"AT&T <tag> test",
|
||||
)
|
||||
)
|
||||
assert result["success"] is True
|
||||
sent_text = send.await_args.args[2]
|
||||
assert "&amp;" not in sent_text
|
||||
assert "&lt;" not in sent_text
|
||||
assert "AT&T" in sent_text
|
||||
|
||||
def test_slack_url_with_parens_formatted_before_send(self, monkeypatch):
|
||||
"""Wikipedia-style URL with parens survives tool-layer formatting."""
|
||||
_ensure_slack_mock(monkeypatch)
|
||||
import gateway.platforms.slack as slack_mod
|
||||
monkeypatch.setattr(slack_mod, "SLACK_AVAILABLE", True)
|
||||
send = AsyncMock(return_value={"success": True, "message_id": "1"})
|
||||
with patch("tools.send_message_tool._send_slack", send):
|
||||
result = asyncio.run(
|
||||
_send_to_platform(
|
||||
Platform.SLACK,
|
||||
SimpleNamespace(enabled=True, token="***", extra={}),
|
||||
"C123",
|
||||
"See [Foo](https://en.wikipedia.org/wiki/Foo_(bar))",
|
||||
)
|
||||
)
|
||||
assert result["success"] is True
|
||||
sent_text = send.await_args.args[2]
|
||||
assert "<https://en.wikipedia.org/wiki/Foo_(bar)|Foo>" in sent_text
|
||||
|
||||
def test_telegram_media_attaches_to_last_chunk(self):
|
||||
"""When chunked, media files are sent only with the last chunk."""
|
||||
|
||||
sent_calls = []
|
||||
|
||||
async def fake_send(token, chat_id, message, media_files=None, thread_id=None):
|
||||
|
||||
Reference in New Issue
Block a user