mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-23 16:36:23 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(slack): fall back to chat.postEphemeral, never public channel, for slash replies

Browse source 
Per review (Victor): response_url failure does not mean ephemeral delivery
is impossible — chat.postEphemeral is an independent API path that keeps
the reply private. Public-channel fallback removed entirely; when both
ephemeral paths fail the reply is dropped with a logged error rather than
leaked to the channel. No config knob needed.
This commit is contained in:
Teknium 2026-07-22 14:54:20 -07:00
parent 79896788f3
commit 700c8bfc5c
2 changed files with 148 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

85
plugins/platforms/slack/adapter.py
View file

@ -1241,6 +1241,56 @@ class SlackAdapter(BasePlatformAdapter):
)
return SendResult(success=False, error=str(e))
async def _post_ephemeral_fallback(
self,
chat_id: str,
ctx: Dict[str, Any],
content: str,
) -> "SendResult":
"""Deliver a slash reply via ``chat.postEphemeral``.
Fallback for when the ``response_url`` POST fails (#19688).
``chat.postEphemeral`` is an independent Web API path that keeps the
reply private to the invoking user — unlike a public channel post,
which must never happen for a reply the user expects to be ephemeral.
Unlike response_url, this cannot ``replace_original``, so the
"Running /cmd…" ack stays; the reply arrives as new ephemeral
message(s) below it. Chunked like normal sends; no 5-POST cap
applies to postEphemeral.
"""
user_id = ctx.get("user_id", "")
if not user_id:
return SendResult(
success=False,
error="no user_id in slash context for postEphemeral",
)
formatted = self.format_message(content)
chunks = self.truncate_message(formatted, self.MAX_MESSAGE_LENGTH)
if not chunks:
chunks = [formatted]
try:
client = self._get_client(chat_id)
for chunk in chunks:
result = await client.chat_postEphemeral(
channel=chat_id,
user=user_id,
text=chunk,
)
if not (isinstance(result, dict) and result.get("ok")):
err = (
result.get("error", "unknown_error")
if isinstance(result, dict)
else "unexpected_response"
)
return SendResult(
success=False,
error=f"chat.postEphemeral failed: {err}",
)
return SendResult(success=True, message_id=None)
except Exception as e:
return SendResult(success=False, error=str(e))
def _warn_if_missing_group_dm_scopes(self, auth_response, team_name: str) -> None:
"""Nudge existing installs to reinstall when group-DM scopes are absent.
@ -1853,15 +1903,35 @@ class SlackAdapter(BasePlatformAdapter):
)
if ephemeral_result.success:
return ephemeral_result
# Ephemeral delivery failed (#19688): fall through to normal
# channel delivery so the command reply is never silently
# dropped. The stale "Running /cmd…" ack remains, but the
# user still gets the actual answer.
# response_url delivery failed (#19688): fall back to
# chat.postEphemeral — an independent API path that keeps
# the reply private ("Only visible to you"). We do NOT fall
# back to a public channel post: a slash reply the user
# expects to be ephemeral must never surface to the whole
# channel just because a delivery path failed.
logger.warning(
"[Slack] Ephemeral slash reply failed (%s); falling back "
"to channel delivery",
"[Slack] response_url slash reply failed (%s); retrying "
"via chat.postEphemeral",
ephemeral_result.error,
)
fallback_result = await self._post_ephemeral_fallback(
chat_id,
slash_ctx,
content,
)
if fallback_result.success:
return fallback_result
# Both ephemeral paths failed — surface the failure instead
# of leaking the reply publicly. The user still has the
# "Running /cmd…" ack; the error is logged and returned so
# the gateway can react (retry surfacing happens upstream).
logger.error(
"[Slack] Ephemeral slash reply failed on both "
"response_url and chat.postEphemeral (%s); dropping "
"rather than posting publicly",
fallback_result.error,
)
return fallback_result
# Convert standard markdown → Slack mrkdwn
formatted = self.format_message(content)
@ -5913,6 +5983,9 @@ class SlackAdapter(BasePlatformAdapter):
if response_url and user_id and channel_id and text.startswith("/"):
self._slash_command_contexts[(channel_id, user_id)] = {
"response_url": response_url,
# Kept for the chat.postEphemeral fallback when response_url
# delivery fails — postEphemeral needs an explicit user.
"user_id": user_id,
"ts": time.monotonic(),
}

74
tests/gateway/test_slack.py
View file

@ -5520,12 +5520,14 @@ class TestSlashEphemeralAck:
@pytest.mark.asyncio
async def test_send_slash_ephemeral_fallback_on_post_failure(self, adapter):
"""Failed response_url POST falls back to normal channel delivery (#19688)."""
"""Failed response_url POST falls back to chat.postEphemeral — never
a public channel post (#19688)."""
import time
from plugins.platforms.slack.adapter import _slash_user_id
adapter._slash_command_contexts[("C1", "U1")] = {
"response_url": "https://hooks.slack.com/commands/bad",
"user_id": "U1",
"ts": time.monotonic(),
}
@ -5544,6 +5546,9 @@ class TestSlashEphemeralAck:
adapter._app.client.chat_postMessage = AsyncMock(
return_value={"ts": "1234.5678", "ok": True}
)
adapter._app.client.chat_postEphemeral = AsyncMock(
return_value={"ok": True}
)
token = _slash_user_id.set("U1")
try:
@ -5554,18 +5559,69 @@ class TestSlashEphemeralAck:
finally:
_slash_user_id.reset(token)
# Reply must not be silently dropped: channel fallback delivered it.
# Reply delivered ephemerally via postEphemeral; the public
# chat.postMessage path must NOT be used for a slash reply.
assert result.success is True
adapter._app.client.chat_postMessage.assert_awaited_once()
adapter._app.client.chat_postEphemeral.assert_awaited_once()
adapter._app.client.chat_postMessage.assert_not_awaited()
@pytest.mark.asyncio
async def test_send_slash_ephemeral_both_paths_fail_never_posts_publicly(
self, adapter
):
"""When response_url AND chat.postEphemeral both fail, the reply is
dropped with an error — never leaked to the public channel (#19688)."""
import time
from plugins.platforms.slack.adapter import _slash_user_id
adapter._slash_command_contexts[("C1", "U1")] = {
"response_url": "https://hooks.slack.com/commands/bad",
"user_id": "U1",
"ts": time.monotonic(),
}
mock_resp = AsyncMock()
mock_resp.status = 500
mock_resp.content = None
mock_resp.text = AsyncMock(return_value="Internal Server Error")
mock_resp.__aenter__ = AsyncMock(return_value=mock_resp)
mock_resp.__aexit__ = AsyncMock(return_value=False)
mock_session = AsyncMock()
mock_session.post = MagicMock(return_value=mock_resp)
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=False)
adapter._app.client.chat_postMessage = AsyncMock(
return_value={"ts": "1234.5678", "ok": True}
)
adapter._app.client.chat_postEphemeral = AsyncMock(
return_value={"ok": False, "error": "channel_not_found"}
)
token = _slash_user_id.set("U1")
try:
with patch(
"plugins.platforms.slack.adapter.aiohttp.ClientSession", return_value=mock_session
):
result = await adapter.send("C1", "Some response")
finally:
_slash_user_id.reset(token)
assert result.success is False
assert "postEphemeral" in (result.error or "")
adapter._app.client.chat_postMessage.assert_not_awaited()
@pytest.mark.asyncio
async def test_send_slash_ephemeral_fallback_on_exception(self, adapter):
"""aiohttp exception on response_url falls back to channel delivery (#19688)."""
"""aiohttp exception on response_url falls back to chat.postEphemeral,
not to public channel delivery (#19688)."""
import time
from plugins.platforms.slack.adapter import _slash_user_id
adapter._slash_command_contexts[("C1", "U1")] = {
"response_url": "https://hooks.slack.com/commands/timeout",
"user_id": "U1",
"ts": time.monotonic(),
}
@ -5577,6 +5633,9 @@ class TestSlashEphemeralAck:
adapter._app.client.chat_postMessage = AsyncMock(
return_value={"ts": "1234.5678", "ok": True}
)
adapter._app.client.chat_postEphemeral = AsyncMock(
return_value={"ok": True}
)
token = _slash_user_id.set("U1")
try:
@ -5588,7 +5647,8 @@ class TestSlashEphemeralAck:
_slash_user_id.reset(token)
assert result.success is True
adapter._app.client.chat_postMessage.assert_awaited_once()
adapter._app.client.chat_postEphemeral.assert_awaited_once()
adapter._app.client.chat_postMessage.assert_not_awaited()
@pytest.mark.asyncio
async def test_send_slash_ephemeral_multichunk_delivers_all_parts(self, adapter):
@ -5703,6 +5763,7 @@ class TestSlashEphemeralAck:
adapter._slash_command_contexts[("C1", "U1")] = {
"response_url": "https://hooks.slack.com/commands/oversized",
"user_id": "U1",
"ts": time.monotonic(),
}
response = _FakeResponse(
@ -5717,6 +5778,9 @@ class TestSlashEphemeralAck:
adapter._app.client.chat_postMessage = AsyncMock(
return_value={"ts": "1234.5678", "ok": True}
)
adapter._app.client.chat_postEphemeral = AsyncMock(
return_value={"ok": True}
)
from plugins.platforms.slack.adapter import _slash_user_id