Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions gateway/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,8 @@ def _merge_platform_map(source_platforms: Any) -> None:
bridged["reply_in_thread"] = platform_cfg["reply_in_thread"]
if "require_mention" in platform_cfg:
bridged["require_mention"] = platform_cfg["require_mention"]
if plat == Platform.SLACK and "strip_bot_mentions" in platform_cfg:
bridged["strip_bot_mentions"] = platform_cfg["strip_bot_mentions"]
if plat == Platform.TELEGRAM and "allowed_chats" in platform_cfg:
bridged["allowed_chats"] = platform_cfg["allowed_chats"]
if plat == Platform.TELEGRAM and "group_allowed_chats" in platform_cfg:
Expand Down Expand Up @@ -1037,6 +1039,8 @@ def _merge_platform_map(source_platforms: Any) -> None:
os.environ["SLACK_REQUIRE_MENTION"] = str(slack_cfg["require_mention"]).lower()
if "strict_mention" in slack_cfg and not os.getenv("SLACK_STRICT_MENTION"):
os.environ["SLACK_STRICT_MENTION"] = str(slack_cfg["strict_mention"]).lower()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main delegates Slack YAML-to-environment translation to plugins/platforms/slack/adapter.py::_apply_yaml_config (gateway/config.py:1269-1309). Add this bridge to that plugin hook rather than the removed core Slack configuration block.

if "strip_bot_mentions" in slack_cfg and not os.getenv("SLACK_STRIP_BOT_MENTIONS"):
os.environ["SLACK_STRIP_BOT_MENTIONS"] = str(slack_cfg["strip_bot_mentions"]).lower()
if "allow_bots" in slack_cfg and not os.getenv("SLACK_ALLOW_BOTS"):
os.environ["SLACK_ALLOW_BOTS"] = str(slack_cfg["allow_bots"]).lower()
frc = slack_cfg.get("free_response_channels")
Expand Down
25 changes: 22 additions & 3 deletions gateway/platforms/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -2522,9 +2522,10 @@ async def _handle_slack_message(self, event: dict) -> None:
):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main migrated the active Slack adapter to plugins/platforms/slack/adapter.py in 5600105478ffde29d7566b45421b100eaa29c4ef; salvage this guard and the other stripping sites into that plugin path so the change reaches the live adapter.

return

if is_mentioned:
if is_mentioned and self._slack_strip_bot_mentions():
# Strip the bot mention from the text
text = text.replace(f"<@{bot_uid}>", "").strip()
if is_mentioned:
# Register this thread so all future messages auto-trigger the bot.
# Skipped in strict mode: strict_mention=true bots must be
# re-mentioned every turn, so remembering the thread would
Expand Down Expand Up @@ -3410,7 +3411,7 @@ async def _fetch_thread_context(
continue

# Strip bot mentions from context messages
if bot_uid:
if bot_uid and self._slack_strip_bot_mentions():
msg_text = msg_text.replace(f"<@{bot_uid}>", "").strip()

prefix = "[thread parent] " if is_parent else ""
Expand Down Expand Up @@ -3480,7 +3481,7 @@ async def _fetch_thread_parent_text(
return ""
bot_uid = self._team_bot_user_ids.get(team_id, self._bot_user_id)
text = (parent.get("text") or "").strip()
if bot_uid:
if bot_uid and self._slack_strip_bot_mentions():
text = text.replace(f"<@{bot_uid}>", "").strip()
return text
except Exception as exc: # pragma: no cover - defensive
Expand Down Expand Up @@ -3780,6 +3781,24 @@ def _slack_strict_mention(self) -> bool:
"on",
}

def _slack_strip_bot_mentions(self) -> bool:
"""Return whether Slack bot mention tokens are removed from prompt text.

Defaults to True to preserve existing behavior. Explicit false values
keep the raw ``<@U...>`` tokens in current and fetched thread text.
"""
configured = self.config.extra.get("strip_bot_mentions")
if configured is not None:
if isinstance(configured, str):
return configured.lower() not in {"false", "0", "no", "off"}
return bool(configured)
return os.getenv("SLACK_STRIP_BOT_MENTIONS", "true").lower() not in {
"false",
"0",
"no",
"off",
}

def _slack_free_response_channels(self) -> set:
"""Return channel IDs where no @mention is required."""
raw = self.config.extra.get("free_response_channels")
Expand Down
1 change: 1 addition & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ def _looks_like_credential(name: str) -> bool:
# Force-clear on every test setup so the leak can't happen.
"SLACK_REQUIRE_MENTION",
"SLACK_STRICT_MENTION",
"SLACK_STRIP_BOT_MENTIONS",
"SLACK_FREE_RESPONSE_CHANNELS",
"SLACK_ALLOW_BOTS",
"SLACK_REACTIONS",
Expand Down
56 changes: 55 additions & 1 deletion tests/gateway/test_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -1801,6 +1801,21 @@ async def test_channel_mention_strips_bot_id(self, adapter):
assert msg_event.text == "what's the weather?"
assert "<@U_BOT>" not in msg_event.text

@pytest.mark.asyncio
async def test_channel_mention_preserves_bot_id_when_config_false(self, adapter):
"""strip_bot_mentions=false keeps the mention token in MessageEvent text."""
adapter.config.extra["strip_bot_mentions"] = False
event = {
"text": "<@U_BOT> what's the weather?",
"user": "U_USER",
"channel": "C123",
"channel_type": "channel",
"ts": "1234567890.000001",
}
await adapter._handle_slack_message(event)
msg_event = adapter.handle_message.call_args[0][0]
assert msg_event.text == "<@U_BOT> what's the weather?"

@pytest.mark.asyncio
async def test_bot_messages_ignored(self, adapter):
"""Messages from bots should be ignored."""
Expand Down Expand Up @@ -3474,7 +3489,7 @@ async def test_slack_reply_to_text_set_on_thread_reply(self, adapter):
{
"ts": "1000.0",
"bot_id": "B_CRON",
"text": "メール要約: 新着メール3件あります",
"text": "<@U_BOT> メール要約: 新着メール3件あります",
},
{"ts": "1000.5", "user": "U_USER", "text": "詳細を教えて"},
]
Expand Down Expand Up @@ -3505,6 +3520,45 @@ async def test_slack_reply_to_text_set_on_thread_reply(self, adapter):
# gateway can inject it when not already in the session history.
assert msg_event.reply_to_text is not None
assert "メール要約" in msg_event.reply_to_text
assert "<@U_BOT>" not in msg_event.reply_to_text

@pytest.mark.asyncio
async def test_slack_reply_to_text_preserves_bot_mention_when_config_false(
self, adapter
):
adapter.config.extra["strip_bot_mentions"] = False
adapter._channel_team = {}
adapter._team_bot_user_ids = {}

adapter._app.client.conversations_replies = AsyncMock(
return_value={
"messages": [
{
"ts": "1000.0",
"bot_id": "B_CRON",
"text": "<@U_BOT> Parent summary",
},
{"ts": "1000.5", "user": "U_USER", "text": "details"},
]
}
)

event = {
"text": "details",
"user": "U_USER",
"channel": "D123",
"channel_type": "im",
"ts": "1000.5",
"thread_ts": "1000.0",
}

with patch.object(
adapter, "_resolve_user_name", new=AsyncMock(return_value="Alice")
):
await adapter._handle_slack_message(event)

msg_event = adapter.handle_message.call_args[0][0]
assert msg_event.reply_to_text == "<@U_BOT> Parent summary"

@pytest.mark.asyncio
async def test_slack_reply_to_text_none_for_top_level_message(self, adapter):
Expand Down
60 changes: 59 additions & 1 deletion tests/gateway/test_slack_approval_buttons.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ async def test_fetches_and_formats_context(self):
mock_client.conversations_replies = AsyncMock(return_value={
"messages": [
{"ts": "1000.0", "user": "U1", "text": "This is the parent message"},
{"ts": "1000.1", "user": "U2", "text": "I think we should refactor"},
{"ts": "1000.1", "user": "U2", "text": "<@U_BOT> I think we should refactor"},
{"ts": "1000.2", "user": "U1", "text": "Good idea, <@U_BOT> what do you think?"},
]
})
Expand All @@ -384,6 +384,31 @@ async def test_fetches_and_formats_context(self):
# Bot mention should be stripped from context
assert "<@U_BOT>" not in context

@pytest.mark.asyncio
async def test_fetch_thread_context_preserves_bot_mentions_when_config_false(self):
adapter = _make_adapter()
adapter.config.extra["strip_bot_mentions"] = False
mock_client = adapter._team_clients["T1"]
mock_client.conversations_replies = AsyncMock(return_value={
"messages": [
{"ts": "1000.0", "user": "U1", "text": "<@U_BOT> parent"},
{"ts": "1000.1", "user": "U2", "text": "prior <@U_BOT> reply"},
{"ts": "1000.2", "user": "U1", "text": "current <@U_BOT>"},
]
})
adapter._user_name_cache = {"U1": "Alice", "U2": "Bob"}

context = await adapter._fetch_thread_context(
channel_id="C1",
thread_ts="1000.0",
current_ts="1000.2",
team_id="T1",
)

assert "[thread parent] Alice: <@U_BOT> parent" in context
assert "Bob: prior <@U_BOT> reply" in context
assert "current <@U_BOT>" not in context

@pytest.mark.asyncio
async def test_skips_bot_messages(self):
"""Self-bot child replies are skipped to avoid circular context,
Expand Down Expand Up @@ -607,6 +632,39 @@ async def test_fetch_thread_parent_text_from_cache(self):
# No additional API call
assert mock_client.conversations_replies.await_count == 1

@pytest.mark.asyncio
async def test_fetch_thread_parent_text_strips_bot_mention_by_default(self):
adapter = _make_adapter()
mock_client = adapter._team_clients["T1"]
mock_client.conversations_replies = AsyncMock(return_value={
"messages": [
{"ts": "1000.0", "user": "U1", "text": "<@U_BOT> Parent summary"},
]
})

parent = await adapter._fetch_thread_parent_text(
channel_id="C1", thread_ts="1000.0", team_id="T1"
)

assert parent == "Parent summary"

@pytest.mark.asyncio
async def test_fetch_thread_parent_text_preserves_bot_mention_when_config_false(self):
adapter = _make_adapter()
adapter.config.extra["strip_bot_mentions"] = False
mock_client = adapter._team_clients["T1"]
mock_client.conversations_replies = AsyncMock(return_value={
"messages": [
{"ts": "1000.0", "user": "U1", "text": "<@U_BOT> Parent summary"},
]
})

parent = await adapter._fetch_thread_parent_text(
channel_id="C1", thread_ts="1000.0", team_id="T1"
)

assert parent == "<@U_BOT> Parent summary"


# ===========================================================================
# _has_active_session_for_thread — session key fix (#5833)
Expand Down
58 changes: 57 additions & 1 deletion tests/gateway/test_slack_mention.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,13 @@ def _ensure_slack_mock():
OTHER_CHANNEL_ID = "C9999999999"


def _make_adapter(require_mention=None, strict_mention=None, free_response_channels=None, allowed_channels=None):
def _make_adapter(
require_mention=None,
strict_mention=None,
free_response_channels=None,
allowed_channels=None,
strip_bot_mentions=None,
):
extra = {}
if require_mention is not None:
extra["require_mention"] = require_mention
Expand All @@ -65,6 +71,8 @@ def _make_adapter(require_mention=None, strict_mention=None, free_response_chann
extra["free_response_channels"] = free_response_channels
if allowed_channels is not None:
extra["allowed_channels"] = allowed_channels
if strip_bot_mentions is not None:
extra["strip_bot_mentions"] = strip_bot_mentions

adapter = object.__new__(SlackAdapter)
adapter.platform = Platform.SLACK
Expand Down Expand Up @@ -180,6 +188,32 @@ def test_strict_mention_env_var_fallback(monkeypatch):
assert adapter._slack_strict_mention() is True


# ---------------------------------------------------------------------------
# Tests: _slack_strip_bot_mentions
# ---------------------------------------------------------------------------

def test_strip_bot_mentions_defaults_to_true(monkeypatch):
monkeypatch.delenv("SLACK_STRIP_BOT_MENTIONS", raising=False)
adapter = _make_adapter()
assert adapter._slack_strip_bot_mentions() is True


def test_strip_bot_mentions_false():
adapter = _make_adapter(strip_bot_mentions=False)
assert adapter._slack_strip_bot_mentions() is False


def test_strip_bot_mentions_string_off():
adapter = _make_adapter(strip_bot_mentions="off")
assert adapter._slack_strip_bot_mentions() is False


def test_strip_bot_mentions_env_var_fallback(monkeypatch):
monkeypatch.setenv("SLACK_STRIP_BOT_MENTIONS", "false")
adapter = _make_adapter()
assert adapter._slack_strip_bot_mentions() is False


# ---------------------------------------------------------------------------
# Tests: _slack_free_response_channels
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -516,6 +550,28 @@ def test_config_bridges_slack_strict_mention(monkeypatch, tmp_path):
assert _os.environ["SLACK_STRICT_MENTION"] == "true"


def test_config_bridges_slack_strip_bot_mentions(monkeypatch, tmp_path):
from gateway.config import load_gateway_config

hermes_home = tmp_path / ".hermes"
hermes_home.mkdir()
(hermes_home / "config.yaml").write_text(
"slack:\n"
" strip_bot_mentions: false\n",
encoding="utf-8",
)

monkeypatch.setenv("HERMES_HOME", str(hermes_home))
monkeypatch.delenv("SLACK_STRIP_BOT_MENTIONS", raising=False)

config = load_gateway_config()

assert config is not None
assert config.platforms[Platform.SLACK].extra.get("strip_bot_mentions") is False
import os as _os
assert _os.environ["SLACK_STRIP_BOT_MENTIONS"] == "false"


# ---------------------------------------------------------------------------
# Regression: strict mode must NOT persist mentions into _mentioned_threads
# ---------------------------------------------------------------------------
Expand Down