mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(slack): harden rich table block fallback
This commit is contained in:
parent
961b832c11
commit
870770b31e
4 changed files with 161 additions and 14 deletions
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue