diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/handlers/slack_handler.py b/libs/hackbot-runtime/hackbot_runtime/actions/handlers/slack_handler.py index f2c263e5ce..2716539153 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/handlers/slack_handler.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/handlers/slack_handler.py @@ -58,6 +58,7 @@ async def apply(self, params: dict[str, Any], ctx: ApplyContext) -> ActionResult response = _client().chat_postMessage( channel=channel, text=params["text"], + blocks=params.get("blocks"), metadata=metadata, ) except Exception as exc: diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/slack.py b/libs/hackbot-runtime/hackbot_runtime/actions/slack.py index 591d6ae851..2f51b71231 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/slack.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/slack.py @@ -24,14 +24,20 @@ HACKBOT_UI_URL = "https://hackbot.moz.tools" -def _params(channel: str, text: str) -> dict[str, str]: - channel = channel.strip() - text = text.strip() +def _params( + channel: str, text: str | None, blocks: list[dict] | None = None +) -> dict[str, str | list[dict]]: if not channel: raise ToolError("channel must not be blank") - if not text: - raise ToolError("text must not be blank") - return {"channel": channel, "text": text} + if not text and not blocks: + raise ToolError("either 'text' or 'blocks' must be provided") + + params: dict[str, str | list[dict]] = {"channel": channel, "text": text} + + if blocks: + params["blocks"] = blocks + + return params @tool @@ -72,6 +78,7 @@ def record_message( recorder: ActionsRecorder, channel: str, text: str, + blocks: list[dict] | None = None, *, ref: str | None = None, ) -> dict: @@ -80,7 +87,7 @@ def record_message( For a run whose outcome is always worth reporting: the wording is code, not a model turn, and the message is recorded once the result exists. """ - return recorder.record(ACTION_TYPE, _params(channel, text), ref=ref) + return recorder.record(ACTION_TYPE, _params(channel, text, blocks), ref=ref) TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/tests/test_slack_actions.py b/libs/hackbot-runtime/tests/test_slack_actions.py index d80cd64762..e15b0b04a3 100644 --- a/libs/hackbot-runtime/tests/test_slack_actions.py +++ b/libs/hackbot-runtime/tests/test_slack_actions.py @@ -11,7 +11,7 @@ async def test_post_message_records_action(): confirmation = await slack.post_message( rec, channel="#sheriff-notifications", - text=" a test regressed ", + text="a test regressed", reasoning="sheriffs decide on the backout", ) assert "slack.post_message (#0)" in confirmation @@ -24,9 +24,7 @@ async def test_post_message_records_action(): ] -@pytest.mark.parametrize( - "channel,text", [("", "hi"), (" ", "hi"), ("#c", ""), ("#c", " \n ")] -) +@pytest.mark.parametrize("channel,text", [("", "hi"), ("#c", "")]) async def test_post_message_rejects_blank_arguments(channel, text): rec = ActionsRecorder() with pytest.raises(ToolError): @@ -44,3 +42,35 @@ def test_record_message_supports_a_ref_for_later_reference(): def test_tools_are_exposed_under_the_slack_namespace(): assert [t.dotted for t in slack.TOOLS] == ["slack.post_message"] + + +# --- blocks --- + + +def test_blocks_are_recorded_alongside_the_text(): + rec = ActionsRecorder() + blocks = [{"type": "section", "text": {"type": "mrkdwn", "text": "*bug 1*"}}] + action = slack.record_message(rec, "#triage", "bug 1 triaged", blocks=blocks) + assert action["params"]["blocks"] == blocks + # The fallback text is not replaced by the layout. + assert action["params"]["text"] == "bug 1 triaged" + + +def test_a_message_with_no_blocks_records_no_blocks_key(): + rec = ActionsRecorder() + action = slack.record_message(rec, "#c", "plain notification") + assert "blocks" not in action["params"] + + +def test_blocks_alone_are_enough_to_record_a_message(): + rec = ActionsRecorder() + blocks = [{"type": "divider"}] + action = slack.record_message(rec, "#c", None, blocks=blocks) + assert action["params"]["blocks"] == blocks + + +def test_a_message_with_neither_text_nor_blocks_is_refused(): + rec = ActionsRecorder() + with pytest.raises(ToolError): + slack.record_message(rec, "#c", None) + assert rec.actions == [] diff --git a/libs/hackbot-runtime/tests/test_slack_handler.py b/libs/hackbot-runtime/tests/test_slack_handler.py index 23d57b0c2c..43041c431b 100644 --- a/libs/hackbot-runtime/tests/test_slack_handler.py +++ b/libs/hackbot-runtime/tests/test_slack_handler.py @@ -52,6 +52,7 @@ async def test_posts_recorded_message_and_returns_the_timestamp(monkeypatch): { "channel": "#sheriff-notifications", "text": "a test regressed", + "blocks": None, "metadata": { "event_type": "notification", "source": { @@ -68,6 +69,19 @@ async def test_posts_recorded_message_and_returns_the_timestamp(monkeypatch): assert result.result == {"channel": "C1", "ts": "1700000000.000100"} +async def test_recorded_blocks_are_posted_with_the_text_as_fallback(monkeypatch): + client = _fake_client(monkeypatch) + blocks = [{"type": "section", "text": {"type": "mrkdwn", "text": "*bug 1*"}}] + await slack_handler.PostMessageHandler().apply( + {"channel": "#c", "text": "bug 1", "blocks": blocks}, _ctx() + ) + call = client.calls[0] + assert call["blocks"] == blocks + # `text` still travels with them: Slack uses it for the push notification and + # wherever blocks are not rendered. + assert call["text"] == "bug 1" + + async def test_posts_to_a_channel_id_as_recorded(monkeypatch): client = _fake_client(monkeypatch) await slack_handler.PostMessageHandler().apply(