diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index b08135c3b1d..6626d1f29d0 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -1597,9 +1597,23 @@ class SlackAdapter(BasePlatformAdapter): if broadcast and i == 0: kwargs["reply_broadcast"] = True - last_result = await self._get_client( - chat_id, team_id=self._metadata_team_id(metadata) - ).chat_postMessage(**kwargs) + try: + last_result = await self._get_client( + chat_id, team_id=self._metadata_team_id(metadata) + ).chat_postMessage(**kwargs) + except Exception as e: + if kwargs.get("blocks") and self._is_block_payload_rejection(e): + retry_kwargs = dict(kwargs) + retry_kwargs.pop("blocks", None) + logger.info( + "[Slack] Block Kit payload rejected; retrying send without blocks: %s", + e, + ) + last_result = await self._get_client( + chat_id, team_id=self._metadata_team_id(metadata) + ).chat_postMessage(**retry_kwargs) + else: + raise # Clear Slack Assistant status as soon as the final message is posted. if thread_ts: @@ -1695,9 +1709,26 @@ class SlackAdapter(BasePlatformAdapter): blocks = self._maybe_blocks(content) if blocks: update_kwargs["blocks"] = blocks - await self._get_client( - chat_id, team_id=self._metadata_team_id(metadata) - ).chat_update(**update_kwargs) + try: + await self._get_client( + chat_id, team_id=self._metadata_team_id(metadata) + ).chat_update(**update_kwargs) + except Exception as e: + if update_kwargs.get("blocks") and self._is_block_payload_rejection(e): + retry_kwargs = dict(update_kwargs) + # Explicitly clear any stale blocks when falling back to the + # flat text update path; otherwise Slack can preserve the + # prior block layout for an edited message. + retry_kwargs["blocks"] = [] + logger.info( + "[Slack] Block Kit payload rejected; retrying edit without blocks: %s", + e, + ) + await self._get_client( + chat_id, team_id=self._metadata_team_id(metadata) + ).chat_update(**retry_kwargs) + else: + raise if finalize: await self.stop_typing(chat_id, metadata=metadata) return SendResult(success=True, message_id=message_id) @@ -2149,6 +2180,31 @@ class SlackAdapter(BasePlatformAdapter): # ----- Markdown → mrkdwn conversion ----- + @staticmethod + def _is_block_payload_rejection(error: BaseException) -> bool: + """Return True for Slack errors recoverable by removing ``blocks``. + + Rich Block Kit output is a progressive enhancement over the plain + ``text`` fallback. If Slack rejects the structured payload as invalid + or too large, retrying the same content without blocks is safe and + prevents a formatting bug from dropping the whole response. + """ + recoverable_codes = { + "invalid_blocks", + "msg_too_long", + "too_many_blocks", + } + response = getattr(error, "response", None) + response_get = getattr(response, "get", None) + if callable(response_get): + try: + if response_get("error") in recoverable_codes: + return True + except Exception: + pass + message = str(error) + return any(code in message for code in recoverable_codes) + def _rich_blocks_enabled(self) -> bool: """Whether to render outbound agent messages as Slack Block Kit blocks. diff --git a/plugins/platforms/slack/block_kit.py b/plugins/platforms/slack/block_kit.py index dc33aacbf11..130a4b95ab3 100644 --- a/plugins/platforms/slack/block_kit.py +++ b/plugins/platforms/slack/block_kit.py @@ -285,18 +285,25 @@ def _table_block(rows: List[str], sep_line: str) -> Optional[Block]: return None aligns = _parse_alignment(sep_line) - column_settings: List[Optional[Dict[str, Any]]] = [] + # Slack requires every provided ``column_settings`` entry to be an object. + # Missing trailing entries inherit defaults, so only emit settings through + # the last non-default alignment. Earlier default-left placeholders still + # need explicit valid objects to preserve positional alignment. + last_non_default = -1 for c in range(min(ncols, MAX_TABLE_COLS)): align = aligns[c] if c < len(aligns) else "left" - # Only emit a setting when it differs from the default (left, no wrap); - # use null to skip a column, per the Slack schema. - column_settings.append({"align": align} if align != "left" else None) + if align != "left": + last_non_default = c + column_settings: List[Dict[str, Any]] = [] + for c in range(last_non_default + 1): + align = aligns[c] if c < len(aligns) else "left" + column_settings.append({"align": align}) block: Block = { "type": "table", "rows": [[_rich_text_cell(cell) for cell in row] for row in parsed], } - if any(cs is not None for cs in column_settings): + if column_settings: block["column_settings"] = column_settings return block diff --git a/tests/gateway/test_slack_block_kit.py b/tests/gateway/test_slack_block_kit.py index 2685606664a..a49ed4b78de 100644 --- a/tests/gateway/test_slack_block_kit.py +++ b/tests/gateway/test_slack_block_kit.py @@ -159,11 +159,34 @@ class TestTables: ) blocks = render_blocks(md) cs = blocks[0]["column_settings"] - # left is default -> null; center/right emitted - assert cs[0] is None + # Every provided entry must be a valid Slack column-settings object. + # Left placeholders are explicit only when needed to preserve position. + assert cs[0] == {"align": "left"} assert cs[1] == {"align": "center"} assert cs[2] == {"align": "right"} + def test_default_trailing_column_settings_are_omitted(self): + md = ( + "| L | R | L2 |\n" + "|---|---:|---|\n" + "| 1 | 2 | 3 |" + ) + blocks = render_blocks(md) + assert blocks is not None + cs = blocks[0]["column_settings"] + assert cs == [{"align": "left"}, {"align": "right"}] + assert all(isinstance(item, dict) for item in cs) + + def test_all_default_table_omits_column_settings(self): + md = ( + "| A | B |\n" + "|---|---|\n" + "| 1 | 2 |" + ) + blocks = render_blocks(md) + assert blocks is not None + assert "column_settings" not in blocks[0] + def test_inline_formatting_inside_cells(self): md = ( "| Item | Link |\n" diff --git a/tests/gateway/test_slack_block_kit_adapter.py b/tests/gateway/test_slack_block_kit_adapter.py index 13ab58c5299..afed8411a1b 100644 --- a/tests/gateway/test_slack_block_kit_adapter.py +++ b/tests/gateway/test_slack_block_kit_adapter.py @@ -7,7 +7,7 @@ Verifies the opt-in behaviour contract: * multi-chunk (>39k) messages fall back to plain text """ -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, call import pytest @@ -29,6 +29,17 @@ def _make_adapter(extra=None): RICH_MD = "# Title\n\n- a\n - nested\n\n---\n\nbody text" +RICH_TABLE_MD = ( + "| Item | Status | Note |\n" + "|---|---:|---|\n" + "| Hermes | ok | table |" +) + + +class SlackRejectedBlocks(Exception): + def __init__(self, error="invalid_blocks"): + super().__init__(f"Slack API rejected blocks: {error}") + self.response = {"error": error} class TestSendMessageBlocks: @@ -98,6 +109,29 @@ class TestSendMessageBlocks: assert "blocks" not in client.chat_postMessage.await_args.kwargs + @pytest.mark.asyncio + async def test_block_rejection_retries_send_without_blocks_using_workspace_client(self): + adapter, client = _make_adapter({"rich_blocks": True}) + client.chat_postMessage = AsyncMock( + side_effect=[SlackRejectedBlocks("invalid_blocks"), {"ts": "111.333"}] + ) + + result = await adapter.send( + "C1", RICH_TABLE_MD, metadata={"team_id": "T_SECONDARY"} + ) + + assert result.success is True + assert adapter._get_client.call_args_list == [ + call("C1", team_id="T_SECONDARY"), + call("C1", team_id="T_SECONDARY"), + ] + assert client.chat_postMessage.await_count == 2 + first = client.chat_postMessage.await_args_list[0].kwargs + second = client.chat_postMessage.await_args_list[1].kwargs + assert "blocks" in first and first["blocks"] + assert "blocks" not in second + assert second["text"] + class TestEditMessageBlocks: @pytest.mark.asyncio @@ -128,3 +162,30 @@ class TestEditMessageBlocks: adapter, client = _make_adapter() # rich_blocks off await adapter.edit_message("C1", "111.222", RICH_MD, finalize=True) assert "blocks" not in client.chat_update.await_args.kwargs + + @pytest.mark.asyncio + async def test_block_rejection_retries_edit_without_blocks_using_workspace_client(self): + adapter, client = _make_adapter({"rich_blocks": True}) + client.chat_update = AsyncMock( + side_effect=[SlackRejectedBlocks("invalid_blocks"), {"ts": "111.222"}] + ) + + result = await adapter.edit_message( + "C1", + "111.222", + RICH_TABLE_MD, + finalize=True, + metadata={"team_id": "T_SECONDARY"}, + ) + + assert result.success is True + assert adapter._get_client.call_args_list == [ + call("C1", team_id="T_SECONDARY"), + call("C1", team_id="T_SECONDARY"), + ] + assert client.chat_update.await_count == 2 + first = client.chat_update.await_args_list[0].kwargs + second = client.chat_update.await_args_list[1].kwargs + assert "blocks" in first and first["blocks"] + assert second["blocks"] == [] + assert second["text"]