diff --git a/README.md b/README.md index 56a73533..7f3706ca 100644 --- a/README.md +++ b/README.md @@ -218,7 +218,9 @@ Revisions happen in the same persistent planner session β€” full context preserv ### πŸ”” Notifications -Async communication between agent and human, delivered to both web UI and Telegram. +Async communication between agent and human, delivered to the web UI, Telegram, +and Slack. `notifications.channels` chooses which of them; all three are on by +default and a transport that is off is skipped. - **`notify`** β€” Fire-and-forget alerts (status updates, completions, reminders) - **`ask_user`** β€” Questions with predefined options, rendered as buttons diff --git a/config.example.yaml b/config.example.yaml index b2b0565b..af186d95 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -138,6 +138,15 @@ slack: # doctor/restart affect the host; sessions lists other channels. Opt in. # commands: [sessions, new, stop, reply] +# Where notify, ask_user, and propose_action deliver. The list replaces the +# default rather than adding to it, so name every transport you want. A +# transport that is off costs nothing here. +notifications: + channels: [web, telegram, slack] + # Target conversation for Slack cards. Without this, the first literal ID + # in slack.allow_channels is used; names and globs are not resolved. + # slack_channel_id: "C0456DEF" + # Quiet hours (local timezone) quiet_start: "02:00" quiet_end: "12:00" diff --git a/docs/architecture.md b/docs/architecture.md index e19c30d2..f7e03671 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -93,8 +93,8 @@ Async notification system for agentβ†’user communication: - **`notify` tool** β€” fire-and-forget notifications (status updates, alerts, reminders) - **`ask_user` tool** β€” questions with predefined options (rendered as buttons) + free-text input. Supports blocking mode (`wait=true`) and async mode (answer injected as session message) - **NotificationService** β€” centralized fanout to configurable channels (web + Telegram by default), answer routing, periodic expiry -- **Multi-channel delivery** β€” web UI via `__global__` WebSocket broadcast channel, Telegram via direct bot API with inline keyboard buttons for questions -- **Answer routing** β€” answers from any channel (web UI, Telegram inline button, `/reply` command) are persisted and either unblock a waiting tool or injected as a user message into the originating session +- **Multi-channel delivery** β€” web UI via `__global__` WebSocket broadcast channel, Telegram via direct bot API with inline keyboard buttons, Slack via Block Kit action buttons +- **Answer routing** β€” answers from any channel (web UI, Telegram inline button, Slack button, `/reply` command) are persisted and either unblock a waiting tool or injected as a user message into the originating session - **Web UI** β€” `/notifications` page with status/type filters, inline answer buttons, dismiss, dismiss-all; real-time toast overlay for new notifications; NavRail badge for pending count ### Cron Service (`nerve/cron/`) diff --git a/docs/config.md b/docs/config.md index 8dae5da0..2506ac8c 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1229,6 +1229,22 @@ Slack slash-command payloads have no thread ID. In a shared channel, `new` and `sessions` therefore refuse, while `stop`, `star`, and `unstar` select among that channel's active thread sessions. Commands work normally in DMs. +### Notifications + +Question and approval cards go to Slack by default. + +| Key | Type | Default | Description | +|-----|------|---------|-------------| +| `notifications.channels` | list | `[web, telegram, slack]` | Where `notify`, `ask_user`, and `propose_action` deliver | +| `notifications.slack_channel_id` | string | `""` | Target channel ID; defaults to the first literal ID in `slack.allow_channels` | + +`notifications.channels` replaces the default rather than adding to it, so +list every transport you want. A name nothing delivers to is skipped with a +warning. Slack in the list costs nothing while Slack is off. + +Names and globs are not resolved for the `slack_channel_id` fallback. Without +a literal channel ID, delivery is skipped with a warning. + ## Quiet Hours | Key | Type | Default | Description | diff --git a/nerve/channels/slack.py b/nerve/channels/slack.py index 241784f3..8684a017 100644 --- a/nerve/channels/slack.py +++ b/nerve/channels/slack.py @@ -36,6 +36,7 @@ _MAX_ACTION_ELEMENTS, _SESSIONS_BUTTON_LIMIT, _md_to_slack, + build_notification_blocks, build_sessions_blocks, slack_emoji_name, slack_to_plain, @@ -1025,6 +1026,16 @@ async def _handle_reaction_event(self, event: dict[str, Any]) -> None: return target, original_text = cached + _, thread_ts = parse_target(target) + if not channel_id.startswith("D") and thread_ts is None: + # A shared channel has no conversation-wide session: each thread + # owns one. A message cached at channel level, such as a + # notification card, has no thread for a reaction to join, and + # opening one would write a slack: mapping that the + # session pickers deliberately do not list. A DM is one + # conversation, so it has no thread to require. + return + channel_type = "im" if channel_id.startswith("D") else "channel" if not await self._authorize(user_id, channel_id, channel_type): return @@ -1202,6 +1213,67 @@ async def _post( ) return resp.get("ts") + def _notification_target(self) -> str | None: + """Resolve a concrete conversation from the active config generation.""" + configured = self.config.notifications.slack_channel_id.strip() + if configured: + if is_slack_id(configured) and configured[0] in "CGD": + return configured + logger.warning( + "notifications.slack_channel_id is not a Slack conversation id", + ) + return None + + for entry in self.config.slack.allow_channels: + if is_slack_id(entry) and entry[0] in "CG": + return entry + logger.warning( + "No notifications.slack_channel_id is set and slack.allow_channels " + "has no literal conversation id", + ) + return None + + async def post_notification( + self, + notification_id: str, + text: str, + options: list[tuple[str, str]] | None = None, + ) -> tuple[str, str] | None: + """Render and post one notification using the active Slack config.""" + if not self.is_available: + return None + target = self._notification_target() + if not target: + return None + blocks = build_notification_blocks(text, notification_id, options) + message_id = await self._post(target, text, blocks) + if not message_id: + return None + self._cache_message(message_id, target, text) + return target, message_id + + async def expire_notification( + self, + target: str, + message_id: str, + text: str, + ) -> None: + """Replace a notification card with its expired state.""" + if not self.is_available: + return + channel_id, _ = parse_target(target) + try: + await self._web.chat_update( + channel=channel_id, + ts=message_id, + text=_md_to_slack(text), + blocks=[], + ) + except Exception as exc: + logger.debug( + "Slack expiry edit failed for %s: %s", message_id, exc, + ) + async def send(self, message: OutboundMessage) -> None: """Send a complete message, split to fit Slack's render limit. @@ -1922,15 +1994,17 @@ async def _handle_notification_button( return actor = (payload.get("user") or {}).get("id") or "" - thread_ts = (payload.get("message") or {}).get("thread_ts") or None + # A card is posted at conversation level, so the target recorded for + # it is the bare conversation. Slack fills in thread_ts on any + # message that has replies, so carrying it across from the press + # would stop matching that record the moment somebody replied under + # the card, and every later press would read as already answered. + target = format_target((payload.get("channel") or {}).get("id") or "") result = await self._notification_service.answer_delivered_notification( notification_id, answer, channel="slack", - target=format_target( - (payload.get("channel") or {}).get("id") or "", - thread_ts, - ), + target=target, actor=actor, ) if not result: diff --git a/nerve/config.py b/nerve/config.py index 4bc4f91c..1c288d53 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -2039,8 +2039,11 @@ def from_dict(cls, d: dict) -> AuthConfig: @dataclass class NotificationsConfig: """Async notification delivery settings.""" - channels: list[str] = field(default_factory=lambda: ["web", "telegram"]) + channels: list[str] = field( + default_factory=lambda: ["web", "telegram", "slack"], + ) telegram_chat_id: int | None = None # Target chat; falls back to first allowed_user + slack_channel_id: str = "" # Target conversation; falls back to a literal id in slack.allow_channels default_expiry_hours: int = 48 # Auto-expire unanswered questions max_redeliveries: int = 3 # Per-row cap on snooze/re-delivery cycles priority_prefixes: dict[str, str] = field(default_factory=lambda: { @@ -2057,8 +2060,9 @@ class NotificationsConfig: @_coerced def from_dict(cls, d: dict) -> NotificationsConfig: return cls( - channels=d.get("channels", ["web", "telegram"]), + channels=d.get("channels", ["web", "telegram", "slack"]), telegram_chat_id=d.get("telegram_chat_id"), + slack_channel_id=str(d.get("slack_channel_id") or ""), default_expiry_hours=d.get("default_expiry_hours", 48), max_redeliveries=d.get("max_redeliveries", 3), priority_prefixes=d.get("priority_prefixes", { diff --git a/nerve/db/notifications.py b/nerve/db/notifications.py index 2f01cbed..06864063 100644 --- a/nerve/db/notifications.py +++ b/nerve/db/notifications.py @@ -176,6 +176,22 @@ async def get_notification_delivery( row = await cursor.fetchone() return dict(row) if row else None + async def get_latest_notification_delivery( + self, + notification_id: str, + channel: str, + ) -> dict | None: + """Return the most recent delivery through one transport.""" + async with self.db.execute( + """SELECT * FROM notification_deliveries + WHERE notification_id = ? AND channel = ? + ORDER BY delivered_at DESC, rowid DESC + LIMIT 1""", + (notification_id, channel), + ) as cursor: + row = await cursor.fetchone() + return dict(row) if row else None + async def find_pending_question_for_delivery( self, channel: str, diff --git a/nerve/notifications/service.py b/nerve/notifications/service.py index fc7e6272..ab278f80 100644 --- a/nerve/notifications/service.py +++ b/nerve/notifications/service.py @@ -2,7 +2,7 @@ Coordinates between MCP tools (agent-side), channels (delivery), and the answer routing mechanism (user-side). Supports fire-and-forget notifications, -async questions with multi-channel delivery (web UI + Telegram), and +async questions with multi-channel delivery (web UI + Telegram + Slack), and ``approval``-kind notifications that route to a server-side dispatcher when the user picks an inline option (see ``nerve.notifications.handlers``). """ @@ -850,6 +850,21 @@ async def _deliver(channel_name: str) -> str | None: telegram_message_id=str(msg_id), ) return "telegram" if msg_id else None + elif channel_name == "slack": + msg_id = await self._deliver_slack( + notification_id, session_id, notif_type, + title, body, priority, options, + option_labels=option_labels, + ) + if not msg_id: + return None + return "slack" + else: + logger.warning( + "notifications.channels names %r, which nothing " + "delivers to; notification %s skips it", + channel_name, notification_id, + ) except Exception as e: logger.error( "Failed to deliver %s to %s: %s", @@ -972,14 +987,15 @@ def _get_telegram_bot(self): return None return channel._app.bot - def _build_telegram_text( + def _build_notification_text( self, session_id: str, title: str, body: str, priority: str, ) -> str: - """Compose the Telegram message text for a notification. + """Compose the chat message text for a notification. - Shared by the initial delivery, the re-delivery tick, and the - expiry edit (which rebuilds the original text to append a - status line). + Shared by Telegram and Slack, and within each by the initial + delivery, the re-delivery tick, and the expiry edit (which rebuilds + the original text to append a status line). Markdown is converted + per channel at send time. """ priority_prefix = self.config.notifications.priority_prefixes.get(priority, "") if title: @@ -1014,7 +1030,7 @@ async def _deliver_telegram( if not chat_id: return None - text = self._build_telegram_text(session_id, title, body, priority) + text = self._build_notification_text(session_id, title, body, priority) if notif_type in ("question", "approval") and options: button_labels: list[tuple[str, str]] = [] @@ -1125,6 +1141,107 @@ async def _send_telegram_inline( return str(msg.message_id) + # ------------------------------------------------------------------ # + # Slack delivery # + # ------------------------------------------------------------------ # + + def _get_slack_channel(self): + """Get the connected SlackChannel, or None if unavailable.""" + channel = self.engine.router.get_channel("slack") + if not channel or not getattr(channel, "is_available", False): + return None + return channel + + async def _deliver_slack( + self, + notification_id: str, + session_id: str, + notif_type: str, + title: str, + body: str, + priority: str, + options: list[str] | None, + option_labels: dict[str, str] | None = None, + ) -> str | None: + """Send a notification to Slack, with Block Kit buttons for answers.""" + channel = self._get_slack_channel() + if not channel: + # Slack is in the default channel list, so most installations + # reach here with it switched off. An absent channel is that + # case and stays quiet; a registered one that cannot take + # traffic is worth a line. + if self.engine.router.get_channel("slack") is None: + logger.debug( + "Slack is not running; notification %s skips it", + notification_id, + ) + else: + logger.warning( + "Slack channel not available for notification %s", + notification_id, + ) + return None + + text = self._build_notification_text(session_id, title, body, priority) + + button_options: list[tuple[str, str]] = [] + if notif_type in ("question", "approval") and options: + for value in options: + if notif_type == "approval": + label = ( + (option_labels or {}).get(value) + or value.replace("_", " ").title() + ) + emoji = _APPROVAL_EMOJIS.get(value, "") + rendered = f"{emoji} {label}".strip() if emoji else label + else: + rendered = value + button_options.append((rendered, value)) + + delivery = await channel.post_notification( + notification_id, + text, + button_options or None, + ) + if not delivery: + return None + target, message_id = delivery + await self.db.record_notification_delivery( + notification_id, + "slack", + target=target, + message_id=message_id, + ) + return message_id + + async def _edit_slack_expired(self, notif: dict[str, Any]) -> None: + """Best-effort edit of the Slack card to show it expired. + + Rebuilds the original text from the row and appends the status line, + dropping the now-dead buttons. All failures are swallowed by design. + """ + channel = self._get_slack_channel() + if not channel: + return + delivery = await self.db.get_latest_notification_delivery( + notif["id"], "slack", + ) + if not delivery or not delivery.get("message_id"): + return + + text = self._build_notification_text( + notif["session_id"], + notif.get("title") or "", + notif.get("body") or "", + notif.get("priority") or "normal", + ) + text += "\n\n⏰ Expired unanswered" + await channel.expire_notification( + delivery["target"], + str(delivery["message_id"]), + text, + ) + # ------------------------------------------------------------------ # # Maintenance (called by the periodic background tick) # # ------------------------------------------------------------------ # @@ -1270,8 +1387,9 @@ async def _report_expired(self, rows: list[dict[str, Any]]) -> None: "expiry broadcast failed for %s: %s", notif["id"], exc, ) - # Telegram: mark the card expired, drop dead buttons. + # Chat channels: mark the card expired, drop dead buttons. await self._edit_telegram_expired(notif) + await self._edit_slack_expired(notif) # Approvals: the proposer is the mechanical pipeline, not a # conversation β€” record the expiry in its audit log. @@ -1359,12 +1477,14 @@ async def _edit_telegram_expired(self, notif: dict[str, Any]) -> None: now-dead inline keyboard. Telegram refuses edits on old messages (>48h) β€” all failures are swallowed by design. """ - target = str(notif.get("telegram_chat_id") or "") - delivery = None - if target: - delivery = await self.db.get_notification_delivery( - notif["id"], "telegram", target, - ) + delivery = await self.db.get_latest_notification_delivery( + notif["id"], "telegram", + ) + target = str( + (delivery or {}).get("target") + or notif.get("telegram_chat_id") + or "" + ) message_id = ( (delivery or {}).get("message_id") or notif.get("telegram_message_id") @@ -1378,7 +1498,7 @@ async def _edit_telegram_expired(self, notif: dict[str, Any]) -> None: if not chat_id: return - text = self._build_telegram_text( + text = self._build_notification_text( notif["session_id"], notif.get("title") or "", notif.get("body") or "", diff --git a/tests/test_notification_lifecycle.py b/tests/test_notification_lifecycle.py index 38d3800d..2eb2d23f 100644 --- a/tests/test_notification_lifecycle.py +++ b/tests/test_notification_lifecycle.py @@ -23,7 +23,6 @@ import json from datetime import datetime, timedelta, timezone from pathlib import Path -from typing import Any from unittest.mock import AsyncMock, MagicMock import pytest @@ -173,9 +172,40 @@ async def test_v045_delivery_scope_exists(self, db: Database): cols = {row[1] async for row in cur} assert {"notification_id", "channel", "target", "message_id"} <= cols + async def test_slack_is_a_default_notification_channel(self): + # Cards reach nobody unless the transport is in this list, and the + # list replaces the default rather than extending it. + from nerve.config import NotificationsConfig + + assert NotificationsConfig().channels == ["web", "telegram", "slack"] + assert NotificationsConfig.from_dict({}).channels == [ + "web", "telegram", "slack", + ] + assert NotificationsConfig.from_dict( + {"channels": ["web"]}, + ).channels == ["web"] + @pytest.mark.asyncio class TestScopedAnswers: + async def test_latest_delivery_can_move_between_targets( + self, db: Database, + ): + await db.create_session("s1", source="external") + await db.create_notification("n1", "s1", "question", "Question") + await db.record_notification_delivery( + "n1", "slack", target="C0123ABC", message_id="1.0", + ) + await db.record_notification_delivery( + "n1", "slack", target="C0456DEF", message_id="2.0", + ) + + delivery = await db.get_latest_notification_delivery("n1", "slack") + + assert delivery + assert delivery["target"] == "C0456DEF" + assert delivery["message_id"] == "2.0" + async def test_latest_question_is_scoped_to_delivery_target( self, db: Database, fake_config, fake_engine, patch_broadcaster, ): @@ -220,6 +250,71 @@ async def test_explicit_answer_rejects_a_different_delivery_target( assert (await db.get_notification("n1"))["status"] == "pending" +@pytest.mark.asyncio +class TestSlackDeliveryBoundary: + async def test_service_records_the_reference_returned_by_the_channel( + self, db: Database, fake_config, fake_engine, + ): + await db.create_session("s1", source="external") + await db.create_notification("n1", "s1", "question", "Question") + channel = MagicMock(is_available=True) + channel.post_notification = AsyncMock( + return_value=("C0456DEF", "1.0"), + ) + fake_engine.router.get_channel.return_value = channel + service = NotificationService(fake_config, db, fake_engine) + + message_id = await service._deliver_slack( + "n1", "s1", "question", "Question", "Body", "normal", ["yes"], + ) + + assert message_id == "1.0" + delivery = await db.get_notification_delivery( + "n1", "slack", "C0456DEF", + ) + assert delivery and delivery["message_id"] == "1.0" + options = channel.post_notification.await_args.args[2] + assert options == [("yes", "yes")] + + async def test_quiescing_channel_is_not_used( + self, db: Database, fake_config, fake_engine, + ): + channel = MagicMock(is_available=False) + channel.post_notification = AsyncMock() + fake_engine.router.get_channel.return_value = channel + service = NotificationService(fake_config, db, fake_engine) + + message_id = await service._deliver_slack( + "n1", "s1", "notify", "Notice", "Body", "normal", None, + ) + + assert message_id is None + channel.post_notification.assert_not_awaited() + + async def test_expiry_uses_the_latest_recorded_target( + self, db: Database, fake_config, fake_engine, + ): + await db.create_session("s1", source="external") + await db.create_notification("n1", "s1", "question", "Question") + await db.record_notification_delivery( + "n1", "slack", target="C0123ABC", message_id="1.0", + ) + await db.record_notification_delivery( + "n1", "slack", target="C0456DEF", message_id="2.0", + ) + channel = MagicMock(is_available=True) + channel.expire_notification = AsyncMock() + fake_engine.router.get_channel.return_value = channel + service = NotificationService(fake_config, db, fake_engine) + + await service._edit_slack_expired(await db.get_notification("n1")) + + channel.expire_notification.assert_awaited_once() + args = channel.expire_notification.await_args.args + assert args[:2] == ("C0456DEF", "2.0") + assert args[2].endswith("⏰ Expired unanswered") + + # ---------------------------------------------------------------------- # Snooze semantics # ---------------------------------------------------------------------- @@ -613,3 +708,123 @@ async def test_telegram_edit_failure_is_swallowed( assert notif["status"] == "expired" # HTML attempt + plain-text fallback, both swallowed. assert bot.edit_message_text.await_count == 2 + + +# ---------------------------------------------------------------------- +# Answer attribution +# ---------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestAnswerAttribution: + """A shared workspace needs to know which member approved an action. + + ``answered_by`` names the transport and the injection path routes on + it, so the person travels beside it in the row's metadata rather than + inside the same string. + """ + + async def test_a_question_answer_records_the_actor( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("s1") + svc = NotificationService(fake_config, db, fake_engine) + result = await svc.ask_question(session_id="s1", title="pick one") + + assert await svc.handle_answer( + result["notification_id"], "yes", "slack", actor="U0123ABC", + ) + notif = await db.get_notification(result["notification_id"]) + assert notif["answered_by"] == "slack" + assert json.loads(notif["metadata"])["answered_by_actor"] == "U0123ABC" + + async def test_the_transport_still_names_the_channel_on_its_own( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + # engine.run is handed answered_by as the channel name, so folding + # the member id into that string would route the reply nowhere. + await db.create_session("s1") + svc = NotificationService(fake_config, db, fake_engine) + result = await svc.ask_question(session_id="s1", title="pick one") + + await svc.handle_answer( + result["notification_id"], "yes", "slack", actor="U0123ABC", + ) + await asyncio.sleep(0) + kwargs = fake_engine.run.call_args.kwargs + assert kwargs["channel"] == "slack" + assert kwargs["source"] == "notification:slack" + + async def test_an_answer_with_no_actor_leaves_the_metadata_alone( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("s1") + svc = NotificationService(fake_config, db, fake_engine) + result = await svc.ask_question(session_id="s1", title="pick one") + + await svc.handle_answer(result["notification_id"], "yes", "web") + notif = await db.get_notification(result["notification_id"]) + assert "answered_by_actor" not in json.loads(notif["metadata"]) + + async def test_existing_metadata_survives_the_answer( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + # The column also carries option_labels, which the Telegram and web + # renderers read after the row is answered. + await db.create_session("s1") + await db.create_notification( + notification_id="n1", session_id="s1", type="question", + title="t", metadata={"option_labels": {"yes": "Ship it"}}, + ) + svc = NotificationService(fake_config, db, fake_engine) + + await svc.handle_answer("n1", "yes", "slack", actor="U0123ABC") + metadata = json.loads((await db.get_notification("n1"))["metadata"]) + assert metadata["option_labels"] == {"yes": "Ship it"} + assert metadata["answered_by_actor"] == "U0123ABC" + + async def test_the_actor_reaches_the_web_broadcast( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("s1") + svc = NotificationService(fake_config, db, fake_engine) + result = await svc.ask_question(session_id="s1", title="pick one") + + await svc.handle_answer( + result["notification_id"], "yes", "slack", actor="U0123ABC", + ) + answered = [ + m for _, m in patch_broadcaster + if m.get("type") == "notification_answered" + ] + assert answered + assert answered[-1]["answered_by"] == "slack" + assert answered[-1]["answered_by_actor"] == "U0123ABC" + + async def test_an_approval_audit_record_names_the_actor( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + audit_workspace, + ): + await db.create_session("s1") + _handlers.register("attribution-test", lambda *a: _handlers.DispatchResult( + ok=True, + audit_event={ + "event": "approval-acted", + "target_kind": "attribution-test", + "decision": "approve", + "ok": True, + }, + )) + svc = NotificationService(fake_config, db, fake_engine) + nid = await _make_approval( + svc, db, target_kind="attribution-test", + ) + + await svc.handle_answer(nid, "approve", "slack", actor="U0123ABC") + records = read_audit_jsonl( + audit_workspace / ".nerve" / "mechanical-actions", + ) + acted = [r for r in records if r.get("event") == "approval-acted"] + assert acted + assert acted[-1]["answered_by"] == "slack" + assert acted[-1]["answered_by_actor"] == "U0123ABC" diff --git a/tests/test_slack_channel.py b/tests/test_slack_channel.py index ae0e2462..a0c8d524 100644 --- a/tests/test_slack_channel.py +++ b/tests/test_slack_channel.py @@ -242,6 +242,53 @@ def test_section_text_stays_inside_slacks_limit(self): assert len(blocks[0]["text"]["text"]) <= 3000 +class TestNotificationDelivery: + @pytest.mark.asyncio + async def test_the_channel_owns_target_resolution_and_block_rendering(self): + channel = _channel(allow_channels=["engineering-*", "C0456DEF"]) + + delivery = await channel.post_notification( + "n1", "Deploy?", [("Approve", "approve")], + ) + + assert delivery == ("C0456DEF", "1.1") + posted = channel._web.chat_postMessage.await_args.kwargs + assert posted["channel"] == "C0456DEF" + assert posted["blocks"][1]["elements"][0]["action_id"] == ( + "notif:n1:approve" + ) + + @pytest.mark.asyncio + async def test_an_explicit_dm_is_a_notification_target(self): + channel = _channel() + channel.config.notifications.slack_channel_id = "D0123ABC" + + delivery = await channel.post_notification("n1", "Hello") + + assert delivery == ("D0123ABC", "1.1") + + @pytest.mark.asyncio + async def test_a_quiescing_channel_refuses_external_delivery(self): + channel = _channel(allow_channels=["C0456DEF"]) + channel._state = "quiescing" + + assert await channel.post_notification("n1", "Hello") is None + channel._web.chat_postMessage.assert_not_awaited() + + @pytest.mark.asyncio + async def test_expiry_replaces_the_card_without_buttons(self): + channel = _channel() + + await channel.expire_notification("C0456DEF", "1.1", "Expired") + + channel._web.chat_update.assert_awaited_once_with( + channel="C0456DEF", + ts="1.1", + text="Expired", + blocks=[], + ) + + # ---------------------------------------------------------------------- # # Channel wiring # # ---------------------------------------------------------------------- # @@ -668,6 +715,36 @@ async def test_a_reaction_does_not_cross_conversations_on_a_shared_ts(self): }) channel.router.handle_message.assert_not_called() + @pytest.mark.asyncio + async def test_a_reaction_on_a_notification_card_opens_no_session(self): + # A card is posted at channel level, so routing a reaction on it + # would write a slack: mapping. Shared channels have no + # conversation-wide session, and the pickers do not list one, so + # nothing could stop it afterwards. + channel = _channel(allow_channels=["C0123ABCD"]) + channel.config.notifications.slack_channel_id = "C0123ABCD" + posted = await channel.post_notification("n1", "Approve this?", None) + assert posted == ("C0123ABCD", "1.1") + + await channel._handle_reaction_event({ + "type": "reaction_added", "user": "U1", "reaction": "eyes", + "item": {"channel": "C0123ABCD", "ts": "1.1"}, + }) + channel.router.handle_message.assert_not_called() + + @pytest.mark.asyncio + async def test_a_reaction_on_a_direct_message_card_still_answers(self): + # A DM is one conversation, so a card there has a session to join. + channel = _channel(allow_users=["U1"], allow_direct_messages=True) + channel.config.notifications.slack_channel_id = "D0123ABCD" + assert await channel.post_notification("n1", "Approve this?", None) + await channel._handle_reaction_event({ + "type": "reaction_added", "user": "U1", "reaction": "eyes", + "item": {"channel": "D0123ABCD", "ts": "1.1"}, + }) + msg = channel.router.handle_message.await_args[0][0] + assert msg.channel_key == "slack:D0123ABCD" + @pytest.mark.asyncio async def test_a_reaction_still_reaches_its_own_thread_session(self): channel = _channel(allow_channels=["C1"]) @@ -2026,7 +2103,9 @@ async def test_a_split_card_keeps_every_section(self, monkeypatch): class TestApprovalAttribution: """`answered_by="slack"` alone loses which member pressed the button.""" - async def _press(self, action_id="notif:n1:approve", value="approve"): + async def _press( + self, action_id="notif:n1:approve", value="approve", **message, + ): channel = _channel(allow_users=["U0123ABC"]) channel._replace_via_url = AsyncMock() service = MagicMock() @@ -2044,11 +2123,29 @@ async def _press(self, action_id="notif:n1:approve", value="approve"): "channel": {"id": "C1"}, "response_url": "https://hooks.slack.test/x", "actions": [{"action_id": action_id, "value": value}], - "message": {"blocks": build_notification_blocks("Ship it?", "n1")}, + "message": { + "blocks": build_notification_blocks("Ship it?", "n1"), + **message, + }, } ) return channel, service + @pytest.mark.asyncio + async def test_a_thread_reply_under_the_card_keeps_the_buttons_working(self): + # Slack fills thread_ts in on a message once it has replies, so a + # press after one reply was looking up "C1:1.1" while the delivery + # record held "C1", and every press read as already answered. + _, service = await self._press(ts="1.1", thread_ts="1.1") + kwargs = service.answer_delivered_notification.await_args.kwargs + assert kwargs["target"] == "C1" + + @pytest.mark.asyncio + async def test_the_button_press_carries_the_slack_member_id(self): + _, service = await self._press() + kwargs = service.answer_delivered_notification.await_args.kwargs + assert kwargs["actor"] == "U0123ABC" + @pytest.mark.asyncio async def test_the_settled_card_names_who_answered(self): channel, service = await self._press()