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): stop consuming slash reply context outside slash sends

Browse source 
_pop_slash_context fell back to a channel-only scan when the
_slash_user_id ContextVar was unset (i.e. send() invoked from a
non-slash code path such as a cron delivery or a normal channel reply).
That scan could steal another user's pending slash reply context: the
normal message got swallowed into an ephemeral response_url POST that
replaces the invoker's ack, and the slash invoker's actual reply then
posted publicly. Remove the fallback — when the ContextVar is unset,
match nothing.

Surgical reapply of PR #26788 (originally against gateway/platforms/slack.py).
This commit is contained in:
Soynchux 2026-07-22 08:18:28 -07:00 • committed by Teknium
parent 5c27b9b19b
commit bd707103ef
No known key found for this signature in database
2 changed files with 58 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -1117,9 +1117,9 @@ class SlackAdapter(BasePlatformAdapter):
Uses the ``_slash_user_id`` ContextVar (set in ``_handle_slash_command``)
to match the exact ``(channel_id, user_id)`` key. This prevents a
concurrent slash command from a different user on the same channel from
stealing another user's ephemeral context. Falls back to a
channel-only scan when the ContextVar is unset (e.g. send() called
from a non-slash code path — should not match anything).
stealing another user's ephemeral context. When the ContextVar is
unset (e.g. send() called from a non-slash code path), do not match
anything — otherwise normal sends can steal a pending slash reply.
"""
now = time.monotonic()
# Clean up stale entries on every lookup — dict is small.
@ -1136,16 +1136,7 @@ class SlackAdapter(BasePlatformAdapter):
if uid:
return self._slash_command_contexts.pop((chat_id, uid), None)
# Fallback: channel-only scan (only reachable when ContextVar is
# unset, i.e. send() called outside a slash-command async context).
match_key = None
for key in list(self._slash_command_contexts):
if key[0] == chat_id:
match_key = key
break
if match_key is None:
return None
return self._slash_command_contexts.pop(match_key)
return None
async def _send_slash_ephemeral(
self,

71
tests/gateway/test_slack.py
View file

@ -5218,13 +5218,18 @@ class TestSlashEphemeralAck:
async def test_pop_slash_context_returns_and_removes(self, adapter):
"""_pop_slash_context returns the context and removes it."""
import time
from plugins.platforms.slack.adapter import _slash_user_id
adapter._slash_command_contexts[("C1", "U1")] = {
"response_url": "https://hooks.slack.com/test",
"ts": time.monotonic(),
}
ctx = adapter._pop_slash_context("C1")
token = _slash_user_id.set("U1")
try:
ctx = adapter._pop_slash_context("C1")
finally:
_slash_user_id.reset(token)
assert ctx is not None
assert ctx["response_url"] == "https://hooks.slack.com/test"
# Must be removed after pop
@ -5254,6 +5259,7 @@ class TestSlashEphemeralAck:
async def test_send_uses_response_url_when_context_exists(self, adapter):
"""send() should POST to response_url for slash command replies."""
import time
from plugins.platforms.slack.adapter import _slash_user_id
adapter._slash_command_contexts[("C_SLASH", "U_SLASH")] = {
"response_url": "https://hooks.slack.com/commands/T123/456/abc",
@ -5270,10 +5276,14 @@ class TestSlashEphemeralAck:
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=False)
with patch(
"plugins.platforms.slack.adapter.aiohttp.ClientSession", return_value=mock_session
):
result = await adapter.send("C_SLASH", "Queued for the next turn.")
token = _slash_user_id.set("U_SLASH")
try:
with patch(
"plugins.platforms.slack.adapter.aiohttp.ClientSession", return_value=mock_session
):
result = await adapter.send("C_SLASH", "Queued for the next turn.")
finally:
_slash_user_id.reset(token)
assert result.success is True
# Verify response_url was POSTed to
@ -5303,6 +5313,7 @@ class TestSlashEphemeralAck:
async def test_send_slash_ephemeral_fallback_on_post_failure(self, adapter):
"""_send_slash_ephemeral returns success=True even if POST fails."""
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",
@ -5320,10 +5331,14 @@ class TestSlashEphemeralAck:
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=False)
with patch(
"plugins.platforms.slack.adapter.aiohttp.ClientSession", return_value=mock_session
):
result = await adapter.send("C1", "Some response")
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)
# Still success — the user saw the initial ack already
assert result.success is True
@ -5332,6 +5347,7 @@ class TestSlashEphemeralAck:
async def test_send_slash_ephemeral_fallback_on_exception(self, adapter):
"""_send_slash_ephemeral returns success=True even if aiohttp raises."""
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",
@ -5343,10 +5359,14 @@ class TestSlashEphemeralAck:
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=False)
with patch(
"plugins.platforms.slack.adapter.aiohttp.ClientSession", return_value=mock_session
):
result = await adapter.send("C1", "Some response")
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 True
@ -5460,10 +5480,27 @@ class TestSlashEphemeralAck:
# ContextVar is unset (default=None) — simulates a normal message send.
assert _slash_user_id.get() is None
ctx = adapter._pop_slash_context("C1")
# Fallback scan still finds it (channel-only) — this is fine for
# the normal single-user case; the ContextVar path is the precise one.
# The key invariant is: when the ContextVar IS set, it matches exactly.
assert ctx is not None # fallback path finds the entry
assert ctx is None
assert ("C1", "U1") in adapter._slash_command_contexts
@pytest.mark.asyncio
async def test_send_without_contextvar_preserves_pending_slash_context(self, adapter):
"""Normal channel sends must not consume a pending slash reply context."""
import time
from plugins.platforms.slack.adapter import _slash_user_id
adapter._slash_command_contexts[("C1", "U1")] = {
"response_url": "https://hooks.slack.com/test",
"ts": time.monotonic(),
}
adapter._app.client.chat_postMessage = AsyncMock(return_value={"ts": "1234.5678", "ok": True})
assert _slash_user_id.get() is None
result = await adapter.send("C1", "public follow-up")
assert result.success is True
adapter._app.client.chat_postMessage.assert_awaited_once()
assert ("C1", "U1") in adapter._slash_command_contexts
# ---------------------------------------------------------------------------