From 0ab2d752ffdae211b0f4fd06c8f62cf7eec191a7 Mon Sep 17 00:00:00 2001 From: probepark Date: Fri, 1 May 2026 09:07:39 +0530 Subject: [PATCH] feat(gateway): private notice delivery and Slack format_message fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds platform-level private notice delivery abstraction so operational messages (e.g. sethome prompt) can be sent ephemerally on Slack when configured with `slack.notice_delivery: private`. Changes: - gateway/config.py: _normalize_notice_delivery() + GatewayConfig.get_notice_delivery() with per-platform config bridging - gateway/platforms/base.py: send_private_notice() default implementation (falls through to send()) - gateway/platforms/slack.py: send_private_notice() via chat_postEphemeral - gateway/run.py: _deliver_platform_notice() helper replaces direct adapter.send() for the sethome notice, with private→public fallback - gateway/platforms/slack.py: app_mention handler now forwards to _handle_slack_message (safe due to ts-based dedup) instead of no-op pass, fixing edge-case Slack configs where mentions arrive only as app_mention - gateway/platforms/slack.py format_message: negative lookbehind prevents markdown images (![]()) from becoming broken Slack links; italic regex now requires non-whitespace boundaries so 'a * b * c' stays literal Based on PR #9340 by @probepark. --- gateway/config.py | 25 ++++++++++ gateway/platforms/base.py | 20 ++++++++ gateway/platforms/slack.py | 53 ++++++++++++++++++--- gateway/run.py | 61 +++++++++++++++++------- tests/gateway/test_config.py | 36 ++++++++++++++ tests/gateway/test_notice_delivery.py | 67 +++++++++++++++++++++++++++ tests/gateway/test_slack.py | 32 +++++++++++++ 7 files changed, 269 insertions(+), 25 deletions(-) create mode 100644 tests/gateway/test_notice_delivery.py diff --git a/gateway/config.py b/gateway/config.py index ce7baffac1..6db8e55d84 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -65,6 +65,15 @@ def _normalize_unauthorized_dm_behavior(value: Any, default: str = "pair") -> st return default +def _normalize_notice_delivery(value: Any, default: str = "public") -> str: + """Normalize notice delivery mode to a supported value.""" + if isinstance(value, str): + normalized = value.strip().lower() + if normalized in {"public", "private"}: + return normalized + return default + + # Module-level cache for bundled platform plugin names (lives outside the # enum so it doesn't become an accidental enum member). _Platform__bundled_plugin_names: Optional[set] = None @@ -592,6 +601,17 @@ class GatewayConfig: ) return self.unauthorized_dm_behavior + def get_notice_delivery(self, platform: Optional[Platform] = None) -> str: + """Return the effective notice-delivery mode for a platform.""" + if platform: + platform_cfg = self.platforms.get(platform) + if platform_cfg and "notice_delivery" in platform_cfg.extra: + return _normalize_notice_delivery( + platform_cfg.extra.get("notice_delivery"), + "public", + ) + return "public" + def load_gateway_config() -> GatewayConfig: """ @@ -707,6 +727,11 @@ def load_gateway_config() -> GatewayConfig: platform_cfg.get("unauthorized_dm_behavior"), gw_data.get("unauthorized_dm_behavior", "pair"), ) + if "notice_delivery" in platform_cfg: + bridged["notice_delivery"] = _normalize_notice_delivery( + platform_cfg.get("notice_delivery"), + "public", + ) if "reply_prefix" in platform_cfg: bridged["reply_prefix"] = platform_cfg["reply_prefix"] if "reply_in_thread" in platform_cfg: diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index ea02279706..ef08b05405 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -1593,6 +1593,26 @@ class BasePlatformAdapter(ABC): """ return SendResult(success=False, error="Not supported") + async def send_private_notice( + self, + chat_id: str, + user_id: Optional[str], + content: str, + reply_to: Optional[str] = None, + metadata: Optional[Dict[str, Any]] = None, + ) -> SendResult: + """Send a notice privately when the platform supports it. + + The default implementation falls back to a normal send so callers can + use one code path across platforms. + """ + return await self.send( + chat_id=chat_id, + content=content, + reply_to=reply_to, + metadata=metadata, + ) + async def send_typing(self, chat_id: str, metadata=None) -> None: """ Send a typing indicator. diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 4c6f29e83b..5479b838a7 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -532,12 +532,13 @@ class SlackAdapter(BasePlatformAdapter): async def handle_message_event(event, say): await self._handle_slack_message(event) - # Acknowledge app_mention events to prevent Bolt 404 errors. - # The "message" handler above already processes @mentions in - # channels, so this is intentionally a no-op to avoid duplicates. + # Handle app_mention explicitly. In some Slack app configurations, + # channel mentions arrive only as app_mention events rather than the + # generic message event. Forward them into the normal message + # pipeline so @mentions reliably produce replies. @self._app.event("app_mention") async def handle_app_mention(event, say): - pass + await self._handle_slack_message(event) # File lifecycle events can arrive around snippet uploads even when # the actual user message is what we care about. Ack them so Slack @@ -725,6 +726,42 @@ class SlackAdapter(BasePlatformAdapter): logger.error("[Slack] Send error: %s", e, exc_info=True) return SendResult(success=False, error=str(e)) + async def send_private_notice( + self, + chat_id: str, + user_id: str, + content: str, + reply_to: Optional[str] = None, + metadata: Optional[Dict[str, Any]] = None, + ) -> SendResult: + """Send a Slack ephemeral message visible only to one user.""" + if not self._app: + return SendResult(success=False, error="Not connected") + if not chat_id or not user_id: + return SendResult(success=False, error="chat_id and user_id are required") + + try: + formatted = self.format_message(content) + thread_ts = self._resolve_thread_ts(reply_to, metadata) + kwargs = { + "channel": chat_id, + "user": user_id, + "text": formatted, + "mrkdwn": True, + } + if thread_ts: + kwargs["thread_ts"] = thread_ts + + result = await self._get_client(chat_id).chat_postEphemeral(**kwargs) + return SendResult( + success=True, + message_id=result.get("message_ts") or result.get("ts"), + raw_response=result, + ) + except Exception as e: # pragma: no cover - defensive logging + logger.error("[Slack] Ephemeral send error: %s", e, exc_info=True) + return SendResult(success=False, error=str(e)) + async def edit_message( self, chat_id: str, @@ -1070,7 +1107,7 @@ class SlackAdapter(BasePlatformAdapter): return _ph(f'<{url}|{label}>') text = re.sub( - r'\[([^\]]+)\]\(([^()]*(?:\([^()]*\)[^()]*)*)\)', + r'(? None: + """Deliver a setup/operational notice using platform-specific privacy rules.""" + adapter = self.adapters.get(source.platform) + if not adapter: + return + + config = getattr(self, "config", None) + notice_delivery = "public" + if config and hasattr(config, "get_notice_delivery"): + notice_delivery = config.get_notice_delivery(source.platform) + + metadata = {"thread_id": source.thread_id} if getattr(source, "thread_id", None) else None + if notice_delivery == "private" and getattr(source, "user_id", None): + try: + result = await adapter.send_private_notice( + source.chat_id, + source.user_id, + content, + metadata=metadata, + ) + if getattr(result, "success", False): + return + except Exception: + pass + + await adapter.send(source.chat_id, content, metadata=metadata) + async def _handle_message(self, event: MessageEvent) -> Optional[str]: """ Handle an incoming message from any platform. @@ -5953,24 +5980,22 @@ class GatewayRunner: platform_name = source.platform.value env_key = _home_target_env_var(platform_name) if not os.getenv(env_key): - adapter = self.adapters.get(source.platform) - if adapter: - # Slack dispatches all Hermes commands through a single - # parent slash command `/hermes`; bare `/sethome` is not - # registered and would fail with "app did not respond". - sethome_cmd = ( - "/hermes sethome" - if source.platform == Platform.SLACK - else "/sethome" - ) - await adapter.send( - source.chat_id, - f"📬 No home channel is set for {platform_name.title()}. " - f"A home channel is where Hermes delivers cron job results " - f"and cross-platform messages.\n\n" - f"Type {sethome_cmd} to make this chat your home channel, " - f"or ignore to skip." - ) + # Slack dispatches all Hermes commands through a single + # parent slash command `/hermes`; bare `/sethome` is not + # registered and would fail with "app did not respond". + sethome_cmd = ( + "/hermes sethome" + if source.platform == Platform.SLACK + else "/sethome" + ) + notice = ( + f"📬 No home channel is set for {platform_name.title()}. " + f"A home channel is where Hermes delivers cron job results " + f"and cross-platform messages.\n\n" + f"Type {sethome_cmd} to make this chat your home channel, " + f"or ignore to skip." + ) + await self._deliver_platform_notice(source, notice) # ----------------------------------------------------------------- # Voice channel awareness — inject current voice channel state diff --git a/tests/gateway/test_config.py b/tests/gateway/test_config.py index 0f5a1440b1..3df2a7d50b 100644 --- a/tests/gateway/test_config.py +++ b/tests/gateway/test_config.py @@ -213,6 +213,26 @@ class TestGatewayConfigRoundtrip: restored = GatewayConfig.from_dict({"always_log_local": "false"}) assert restored.always_log_local is False + def test_get_notice_delivery_defaults_to_public(self): + config = GatewayConfig( + platforms={Platform.SLACK: PlatformConfig(enabled=True, token="***")} + ) + + assert config.get_notice_delivery(Platform.SLACK) == "public" + + def test_get_notice_delivery_honors_platform_override(self): + config = GatewayConfig( + platforms={ + Platform.SLACK: PlatformConfig( + enabled=True, + token="***", + extra={"notice_delivery": "private"}, + ), + } + ) + + assert config.get_notice_delivery(Platform.SLACK) == "private" + class TestLoadGatewayConfig: def test_bridges_quick_commands_from_config_yaml(self, tmp_path, monkeypatch): @@ -457,6 +477,22 @@ class TestLoadGatewayConfig: assert config.platforms[Platform.TELEGRAM].extra["disable_link_previews"] is True + def test_bridges_notice_delivery_from_config_yaml(self, tmp_path, monkeypatch): + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + config_path = hermes_home / "config.yaml" + config_path.write_text( + "slack:\n" + " notice_delivery: private\n", + encoding="utf-8", + ) + + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + config = load_gateway_config() + + assert config.get_notice_delivery(Platform.SLACK) == "private" + def test_bridges_telegram_proxy_url_from_config_yaml(self, tmp_path, monkeypatch): hermes_home = tmp_path / ".hermes" hermes_home.mkdir() diff --git a/tests/gateway/test_notice_delivery.py b/tests/gateway/test_notice_delivery.py new file mode 100644 index 0000000000..0f2a22ff96 --- /dev/null +++ b/tests/gateway/test_notice_delivery.py @@ -0,0 +1,67 @@ +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import GatewayConfig, Platform, PlatformConfig +from gateway.platforms.base import SendResult +from gateway.run import GatewayRunner +from gateway.session import SessionSource + + +def _make_source() -> SessionSource: + return SessionSource( + platform=Platform.SLACK, + chat_id="C123", + chat_type="channel", + user_id="U123", + thread_id="111.222", + ) + + +def _make_runner(extra=None): + runner = object.__new__(GatewayRunner) + runner.config = GatewayConfig( + platforms={ + Platform.SLACK: PlatformConfig(enabled=True, token="***", extra=extra or {}) + } + ) + adapter = MagicMock() + adapter.send = AsyncMock(return_value=SendResult(success=True, message_id="public-1")) + adapter.send_private_notice = AsyncMock(return_value=SendResult(success=True, message_id="private-1")) + runner.adapters = {Platform.SLACK: adapter} + return runner, adapter + + +@pytest.mark.asyncio +async def test_deliver_platform_notice_uses_private_delivery_when_configured(): + runner, adapter = _make_runner(extra={"notice_delivery": "private"}) + + await runner._deliver_platform_notice(_make_source(), "hello") + + adapter.send_private_notice.assert_awaited_once_with( + "C123", + "U123", + "hello", + metadata={"thread_id": "111.222"}, + ) + adapter.send.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_deliver_platform_notice_falls_back_to_public_when_private_fails(): + runner, adapter = _make_runner(extra={"notice_delivery": "private"}) + adapter.send_private_notice = AsyncMock(return_value=SendResult(success=False, error="nope")) + + await runner._deliver_platform_notice(_make_source(), "hello") + + adapter.send.assert_awaited_once_with("C123", "hello", metadata={"thread_id": "111.222"}) + + +@pytest.mark.asyncio +async def test_deliver_platform_notice_uses_public_delivery_by_default(): + runner, adapter = _make_runner() + + await runner._deliver_platform_notice(_make_source(), "hello") + + adapter.send.assert_awaited_once_with("C123", "hello", metadata={"thread_id": "111.222"}) + adapter.send_private_notice.assert_not_awaited() diff --git a/tests/gateway/test_slack.py b/tests/gateway/test_slack.py index 5830d1f517..cd455d5fc5 100644 --- a/tests/gateway/test_slack.py +++ b/tests/gateway/test_slack.py @@ -518,6 +518,28 @@ class TestSendDocument: sleep_mock.assert_awaited_once() +class TestSendPrivateNotice: + @pytest.mark.asyncio + async def test_send_private_notice_uses_ephemeral_api(self, adapter): + adapter._app.client.chat_postEphemeral = AsyncMock(return_value={"message_ts": "123.456"}) + + result = await adapter.send_private_notice( + chat_id="C123", + user_id="U123", + content="private hello", + metadata={"thread_id": "1234567890.123456"}, + ) + + assert result.success + adapter._app.client.chat_postEphemeral.assert_called_once_with( + channel="C123", + user="U123", + text="private hello", + mrkdwn=True, + thread_ts="1234567890.123456", + ) + + # --------------------------------------------------------------------------- # TestSendVideo # --------------------------------------------------------------------------- @@ -1315,6 +1337,16 @@ class TestFormatMessage: result = adapter.format_message("[link](https://x.com?a=1&b=2)") assert result == "" + def test_markdown_image_does_not_create_broken_slack_link(self, adapter): + """Markdown image syntax should not become '!' in Slack.""" + result = adapter.format_message("![alt](https://img.example.com/cat.png)") + assert result == "![alt](https://img.example.com/cat.png)" + + def test_literal_asterisks_with_spaces_are_not_treated_as_italic(self, adapter): + """Asterisks used as plain delimiters should stay literal.""" + result = adapter.format_message("a * b * c") + assert result == "a * b * c" + def test_emoji_shortcodes_passthrough(self, adapter): """Emoji shortcodes like :smile: pass through unchanged.""" assert adapter.format_message(":smile: hello :wave:") == ":smile: hello :wave:"