feat(gateway): private notice delivery and Slack format_message fixes
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.
This commit is contained in:
@@ -65,6 +65,15 @@ def _normalize_unauthorized_dm_behavior(value: Any, default: str = "pair") -> st
|
|||||||
return default
|
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
|
# Module-level cache for bundled platform plugin names (lives outside the
|
||||||
# enum so it doesn't become an accidental enum member).
|
# enum so it doesn't become an accidental enum member).
|
||||||
_Platform__bundled_plugin_names: Optional[set] = None
|
_Platform__bundled_plugin_names: Optional[set] = None
|
||||||
@@ -592,6 +601,17 @@ class GatewayConfig:
|
|||||||
)
|
)
|
||||||
return self.unauthorized_dm_behavior
|
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:
|
def load_gateway_config() -> GatewayConfig:
|
||||||
"""
|
"""
|
||||||
@@ -707,6 +727,11 @@ def load_gateway_config() -> GatewayConfig:
|
|||||||
platform_cfg.get("unauthorized_dm_behavior"),
|
platform_cfg.get("unauthorized_dm_behavior"),
|
||||||
gw_data.get("unauthorized_dm_behavior", "pair"),
|
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:
|
if "reply_prefix" in platform_cfg:
|
||||||
bridged["reply_prefix"] = platform_cfg["reply_prefix"]
|
bridged["reply_prefix"] = platform_cfg["reply_prefix"]
|
||||||
if "reply_in_thread" in platform_cfg:
|
if "reply_in_thread" in platform_cfg:
|
||||||
|
|||||||
@@ -1593,6 +1593,26 @@ class BasePlatformAdapter(ABC):
|
|||||||
"""
|
"""
|
||||||
return SendResult(success=False, error="Not supported")
|
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:
|
async def send_typing(self, chat_id: str, metadata=None) -> None:
|
||||||
"""
|
"""
|
||||||
Send a typing indicator.
|
Send a typing indicator.
|
||||||
|
|||||||
@@ -532,12 +532,13 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
async def handle_message_event(event, say):
|
async def handle_message_event(event, say):
|
||||||
await self._handle_slack_message(event)
|
await self._handle_slack_message(event)
|
||||||
|
|
||||||
# Acknowledge app_mention events to prevent Bolt 404 errors.
|
# Handle app_mention explicitly. In some Slack app configurations,
|
||||||
# The "message" handler above already processes @mentions in
|
# channel mentions arrive only as app_mention events rather than the
|
||||||
# channels, so this is intentionally a no-op to avoid duplicates.
|
# generic message event. Forward them into the normal message
|
||||||
|
# pipeline so @mentions reliably produce replies.
|
||||||
@self._app.event("app_mention")
|
@self._app.event("app_mention")
|
||||||
async def handle_app_mention(event, say):
|
async def handle_app_mention(event, say):
|
||||||
pass
|
await self._handle_slack_message(event)
|
||||||
|
|
||||||
# File lifecycle events can arrive around snippet uploads even when
|
# File lifecycle events can arrive around snippet uploads even when
|
||||||
# the actual user message is what we care about. Ack them so Slack
|
# 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)
|
logger.error("[Slack] Send error: %s", e, exc_info=True)
|
||||||
return SendResult(success=False, error=str(e))
|
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(
|
async def edit_message(
|
||||||
self,
|
self,
|
||||||
chat_id: str,
|
chat_id: str,
|
||||||
@@ -1070,7 +1107,7 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
return _ph(f'<{url}|{label}>')
|
return _ph(f'<{url}|{label}>')
|
||||||
|
|
||||||
text = re.sub(
|
text = re.sub(
|
||||||
r'\[([^\]]+)\]\(([^()]*(?:\([^()]*\)[^()]*)*)\)',
|
r'(?<!!)\[([^\]]+)\]\(([^()]*(?:\([^()]*\)[^()]*)*)\)',
|
||||||
_convert_markdown_link,
|
_convert_markdown_link,
|
||||||
text,
|
text,
|
||||||
)
|
)
|
||||||
@@ -1117,9 +1154,11 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
)
|
)
|
||||||
|
|
||||||
# 10) Convert italic: _text_ stays as _text_ (already Slack italic)
|
# 10) Convert italic: _text_ stays as _text_ (already Slack italic)
|
||||||
# Single *text* → _text_ (Slack italic)
|
# Single *text* → _text_ (Slack italic), but only when the
|
||||||
|
# emphasized text touches non-whitespace on both sides so literal
|
||||||
|
# delimiters like "a * b * c" are preserved.
|
||||||
text = re.sub(
|
text = re.sub(
|
||||||
r'(?<!\*)\*([^*\n]+)\*(?!\*)',
|
r'(?<!\*)\*(\S(?:[^*\n]*?\S)?)\*(?!\*)',
|
||||||
lambda m: _ph(f'_{m.group(1)}_'),
|
lambda m: _ph(f'_{m.group(1)}_'),
|
||||||
text,
|
text,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -4294,6 +4294,33 @@ class GatewayRunner:
|
|||||||
|
|
||||||
return "pair"
|
return "pair"
|
||||||
|
|
||||||
|
async def _deliver_platform_notice(self, source, content: str) -> 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]:
|
async def _handle_message(self, event: MessageEvent) -> Optional[str]:
|
||||||
"""
|
"""
|
||||||
Handle an incoming message from any platform.
|
Handle an incoming message from any platform.
|
||||||
@@ -5953,24 +5980,22 @@ class GatewayRunner:
|
|||||||
platform_name = source.platform.value
|
platform_name = source.platform.value
|
||||||
env_key = _home_target_env_var(platform_name)
|
env_key = _home_target_env_var(platform_name)
|
||||||
if not os.getenv(env_key):
|
if not os.getenv(env_key):
|
||||||
adapter = self.adapters.get(source.platform)
|
# Slack dispatches all Hermes commands through a single
|
||||||
if adapter:
|
# parent slash command `/hermes`; bare `/sethome` is not
|
||||||
# Slack dispatches all Hermes commands through a single
|
# registered and would fail with "app did not respond".
|
||||||
# parent slash command `/hermes`; bare `/sethome` is not
|
sethome_cmd = (
|
||||||
# registered and would fail with "app did not respond".
|
"/hermes sethome"
|
||||||
sethome_cmd = (
|
if source.platform == Platform.SLACK
|
||||||
"/hermes sethome"
|
else "/sethome"
|
||||||
if source.platform == Platform.SLACK
|
)
|
||||||
else "/sethome"
|
notice = (
|
||||||
)
|
f"📬 No home channel is set for {platform_name.title()}. "
|
||||||
await adapter.send(
|
f"A home channel is where Hermes delivers cron job results "
|
||||||
source.chat_id,
|
f"and cross-platform messages.\n\n"
|
||||||
f"📬 No home channel is set for {platform_name.title()}. "
|
f"Type {sethome_cmd} to make this chat your home channel, "
|
||||||
f"A home channel is where Hermes delivers cron job results "
|
f"or ignore to skip."
|
||||||
f"and cross-platform messages.\n\n"
|
)
|
||||||
f"Type {sethome_cmd} to make this chat your home channel, "
|
await self._deliver_platform_notice(source, notice)
|
||||||
f"or ignore to skip."
|
|
||||||
)
|
|
||||||
|
|
||||||
# -----------------------------------------------------------------
|
# -----------------------------------------------------------------
|
||||||
# Voice channel awareness — inject current voice channel state
|
# Voice channel awareness — inject current voice channel state
|
||||||
|
|||||||
@@ -213,6 +213,26 @@ class TestGatewayConfigRoundtrip:
|
|||||||
restored = GatewayConfig.from_dict({"always_log_local": "false"})
|
restored = GatewayConfig.from_dict({"always_log_local": "false"})
|
||||||
assert restored.always_log_local is 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:
|
class TestLoadGatewayConfig:
|
||||||
def test_bridges_quick_commands_from_config_yaml(self, tmp_path, monkeypatch):
|
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
|
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):
|
def test_bridges_telegram_proxy_url_from_config_yaml(self, tmp_path, monkeypatch):
|
||||||
hermes_home = tmp_path / ".hermes"
|
hermes_home = tmp_path / ".hermes"
|
||||||
hermes_home.mkdir()
|
hermes_home.mkdir()
|
||||||
|
|||||||
67
tests/gateway/test_notice_delivery.py
Normal file
67
tests/gateway/test_notice_delivery.py
Normal file
@@ -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()
|
||||||
@@ -518,6 +518,28 @@ class TestSendDocument:
|
|||||||
sleep_mock.assert_awaited_once()
|
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
|
# TestSendVideo
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -1315,6 +1337,16 @@ class TestFormatMessage:
|
|||||||
result = adapter.format_message("[link](https://x.com?a=1&b=2)")
|
result = adapter.format_message("[link](https://x.com?a=1&b=2)")
|
||||||
assert result == "<https://x.com?a=1&b=2|link>"
|
assert result == "<https://x.com?a=1&b=2|link>"
|
||||||
|
|
||||||
|
def test_markdown_image_does_not_create_broken_slack_link(self, adapter):
|
||||||
|
"""Markdown image syntax should not become '!<url|alt>' in Slack."""
|
||||||
|
result = adapter.format_message("")
|
||||||
|
assert result == ""
|
||||||
|
|
||||||
|
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):
|
def test_emoji_shortcodes_passthrough(self, adapter):
|
||||||
"""Emoji shortcodes like :smile: pass through unchanged."""
|
"""Emoji shortcodes like :smile: pass through unchanged."""
|
||||||
assert adapter.format_message(":smile: hello :wave:") == ":smile: hello :wave:"
|
assert adapter.format_message(":smile: hello :wave:") == ":smile: hello :wave:"
|
||||||
|
|||||||
Reference in New Issue
Block a user