diff --git a/tests/test_channel_slack.py b/tests/test_channel_slack.py index c97a2e5e..a2179071 100644 --- a/tests/test_channel_slack.py +++ b/tests/test_channel_slack.py @@ -77,9 +77,14 @@ def _make_bot() -> tuple[object, MagicMock, MagicMock]: client.conversations_history = AsyncMock(return_value={"ok": True, "messages": []}) client.views_open = AsyncMock(return_value={"ok": True}) + # Patch httpx.AsyncClient so each test doesn't open a real client that + # leaks an unclosed-warning at GC time. The bot's _http_client is only + # used by the SDK router (which we replace with a MagicMock below), so + # an AsyncMock standin is enough for every test that uses this factory. with ( patch("turnstone.channels.slack.bot.AsyncApp", MagicMock()), patch("turnstone.channels.slack.bot.AsyncWebClient", return_value=client), + patch("turnstone.channels.slack.bot.httpx.AsyncClient", return_value=AsyncMock()), ): bot = TurnstoneSlackBot( config, @@ -193,11 +198,28 @@ class TestPreviewSanitization: text = "<@U123>`abc`" + ("x" * 2000) out = _sanitize_slack_preview(text, max_length=50) + # Mention markup is escaped so it can't render as a real ping assert "<@U123>" in out - assert "`abc`" not in out - assert "\\`abc\\`" in out + # Single backticks survive — code-quoted snippets stay readable + assert "`abc`" in out assert len(out) <= 50 + def test_sanitize_slack_preview_neutralizes_triple_backtick(self) -> None: + """Triple backticks would close the surrounding mrkdwn fence — splice + a zero-width space inside so Slack no longer recognizes it as a + delimiter.""" + from turnstone.channels.slack.bot import _sanitize_slack_preview + + out = _sanitize_slack_preview("inner ``` text", max_length=200) + assert "```" not in out + assert "``\u200b`" in out + + def test_sanitize_slack_preview_keeps_short_input(self) -> None: + from turnstone.channels.slack.bot import _sanitize_slack_preview + + out = _sanitize_slack_preview("short and clean", max_length=200) + assert out == "short and clean" + # --------------------------------------------------------------------------- # StreamingMessage @@ -546,6 +568,10 @@ class TestWsEventDispatch: with ( patch("turnstone.channels.slack.bot.AsyncApp", MagicMock()), patch("turnstone.channels.slack.bot.AsyncWebClient", return_value=client), + patch( + "turnstone.channels.slack.bot.httpx.AsyncClient", + return_value=AsyncMock(), + ), ): bot = TurnstoneSlackBot( config, @@ -622,13 +648,19 @@ class TestWsEventDispatch: with ( patch("turnstone.channels.slack.bot.AsyncApp", MagicMock()), patch("turnstone.channels.slack.bot.AsyncWebClient", return_value=client), + patch( + "turnstone.channels.slack.bot.httpx.AsyncClient", + return_value=AsyncMock(), + ), ): bot = TurnstoneSlackBot(config, server_url="http://localhost:8080", storage=storage) bot.router = router # type: ignore[attr-defined] bot._client = client # type: ignore[attr-defined] bot.storage = None # type: ignore[attr-defined] - event = ApproveRequestEvent(ws_id="ws-1", items=[{"func_name": "bash", "needs_approval": True}]) + event = ApproveRequestEvent( + ws_id="ws-1", items=[{"func_name": "bash", "needs_approval": True}] + ) route = SlackRoute(channel="C1", user_id="U1", thread_ts="123.456") _run(bot._on_ws_event("ws-1", route, event)) # type: ignore[attr-defined] @@ -640,7 +672,9 @@ class TestWsEventDispatch: bot, client = self._make_ws_bot() - event = ApproveRequestEvent(ws_id="ws-1", items=[{"func_name": "bash", "needs_approval": True}]) + event = ApproveRequestEvent( + ws_id="ws-1", items=[{"func_name": "bash", "needs_approval": True}] + ) route = SlackRoute(channel="C1", user_id="U12345", thread_ts="123.456") _run(bot._on_ws_event("ws-1", route, event)) # type: ignore[attr-defined] @@ -732,11 +766,7 @@ class TestWsEventDispatch: view = { "private_metadata": "ws-1", "state": { - "values": { - "feedback_block": { - "feedback_input": {"value": "please revise step 2"} - } - } + "values": {"feedback_block": {"feedback_input": {"value": "please revise step 2"}}} }, } @@ -768,6 +798,10 @@ class TestNotificationTracking: with ( patch("turnstone.channels.slack.bot.AsyncApp", MagicMock()), patch("turnstone.channels.slack.bot.AsyncWebClient"), + patch( + "turnstone.channels.slack.bot.httpx.AsyncClient", + return_value=AsyncMock(), + ), ): bot = TurnstoneSlackBot(config, server_url="http://localhost:8080", storage=storage) @@ -785,6 +819,10 @@ class TestNotificationTracking: with ( patch("turnstone.channels.slack.bot.AsyncApp", MagicMock()), patch("turnstone.channels.slack.bot.AsyncWebClient"), + patch( + "turnstone.channels.slack.bot.httpx.AsyncClient", + return_value=AsyncMock(), + ), ): bot = TurnstoneSlackBot(config, server_url="http://localhost:8080", storage=storage) @@ -843,9 +881,7 @@ class TestNotificationTracking: class TestDmContinuity: def test_dm_messages_reuse_same_workstream(self) -> None: bot, router, _client = _make_bot() - router.get_or_create_workstream = AsyncMock( - side_effect=[("ws-dm", True), ("ws-dm", False)] - ) + router.get_or_create_workstream = AsyncMock(side_effect=[("ws-dm", True), ("ws-dm", False)]) event1 = _make_slack_event( channel="D12345", @@ -900,7 +936,13 @@ class TestChannelCLI: patch.object( sys, "argv", - ["turnstone-channel", "--slack-token", "xoxb-test", "--server-url", "http://localhost:8080"], + [ + "turnstone-channel", + "--slack-token", + "xoxb-test", + "--server-url", + "http://localhost:8080", + ], ), patch.dict("os.environ", {}, clear=True), pytest.raises(SystemExit) as exc_info, @@ -942,6 +984,7 @@ class TestChannelCLI: for aw in aws: await aw return [] + storage = MagicMock() storage.register_service = MagicMock() storage.deregister_service = MagicMock() @@ -962,7 +1005,9 @@ class TestChannelCLI: ), patch("turnstone.core.storage._registry.init_storage"), patch("turnstone.core.storage._registry.get_storage", return_value=storage), - patch("turnstone.channels._http.create_channel_app", side_effect=_fake_create_channel_app), + patch( + "turnstone.channels._http.create_channel_app", side_effect=_fake_create_channel_app + ), patch("turnstone.channels._http._get_service_id", return_value="channel-test"), patch("asyncio.gather", side_effect=_fake_gather), patch("uvicorn.Config", return_value=MagicMock()), @@ -1037,7 +1082,9 @@ class TestChannelCLI: ), patch("turnstone.core.storage._registry.init_storage"), patch("turnstone.core.storage._registry.get_storage", return_value=storage), - patch("turnstone.channels._http.create_channel_app", side_effect=_fake_create_channel_app), + patch( + "turnstone.channels._http.create_channel_app", side_effect=_fake_create_channel_app + ), patch("turnstone.channels._http._get_service_id", return_value="channel-test"), patch("asyncio.gather", side_effect=_fake_gather), patch("uvicorn.Config", return_value=MagicMock()), @@ -1049,4 +1096,4 @@ class TestChannelCLI: assert "discord" in created_adapters assert "slack" in created_adapters - assert len(created_adapters) == 2 \ No newline at end of file + assert len(created_adapters) == 2 diff --git a/turnstone/channels/slack/__init__.py b/turnstone/channels/slack/__init__.py index e69de29b..fb43d19a 100644 --- a/turnstone/channels/slack/__init__.py +++ b/turnstone/channels/slack/__init__.py @@ -0,0 +1,11 @@ +"""Slack channel adapter (Socket Mode). + +Bridges Slack channel mentions, slash-command sessions, and DMs to +turnstone workstreams. The :class:`TurnstoneSlackBot` runs over +slack-bolt's Socket Mode (no public URL or signing-secret needed) +and shares the per-user routing, approvals, and SSE-event consumption +patterns established by the Discord adapter. + +See ``turnstone/channels/cli.py`` for the gateway entry point and +``turnstone/channels/slack/config.py`` for app + token setup. +""" diff --git a/turnstone/channels/slack/bot.py b/turnstone/channels/slack/bot.py index 1a8a5500..734aabcf 100644 --- a/turnstone/channels/slack/bot.py +++ b/turnstone/channels/slack/bot.py @@ -66,20 +66,38 @@ _GREETING = "Hey! Let me know what I can help with." _SSE_RECONNECT_DELAY: float = 2.0 _SSE_MAX_RECONNECT_DELAY: float = 30.0 +# Slack section.text caps at 3000 chars. Reserve headroom for the heading +# and the "+N more" suffix; cap each preview individually so a single huge +# tool can't crowd out the rest of a multi-tool batch. +_APPROVAL_TEXT_BUDGET: int = 2700 +_APPROVAL_PER_ITEM_PREVIEW: int = 600 + + def _sanitize_slack_preview(text: str, max_length: int = 1200) -> str: - """Escape Slack mrkdwn-sensitive content for safe fenced display.""" - text = text.replace("`", "\\`") + """Escape Slack mrkdwn-sensitive content for safe fenced display. + + Targets the *closing-fence* injection vector: any literal ```\ + inside the content would terminate the surrounding mrkdwn fence and + let the rest of the preview render as live markup (mentions, links, + etc.). We splice a zero-width space inside the triple sequence so + Slack no longer recognizes it as a fence delimiter, and escape + ``&<>`` for good measure. Single backticks are kept intact so code + snippets in plans / tool previews remain readable. + """ text = text.replace("&", "&").replace("<", "<").replace(">", ">") + text = text.replace("```", "``\u200b`") if len(text) > max_length: return text[: max_length - 3] + "..." return text + def _parse_ts(ts: str) -> tuple[int, int]: parts = ts.split(".", 1) seconds = int(parts[0]) micros = int(parts[1]) if len(parts) > 1 else 0 return (seconds, micros) + @dataclass(frozen=True) class PendingApproval: channel: str @@ -120,7 +138,7 @@ class StreamingMessage: text=chunks[0], ) except Exception: - log.debug("slack.streaming_message.finalize_edit_failed") + log.debug("slack.streaming_message.finalize_edit_failed", exc_info=True) for chunk in chunks[1:]: await self.client.chat_postMessage( @@ -160,7 +178,7 @@ class StreamingMessage: text=display, ) except Exception: - log.debug("slack.streaming_message.flush_failed") + log.debug("slack.streaming_message.flush_failed", exc_info=True) self._last_edit = time.monotonic() @@ -340,6 +358,7 @@ class TurnstoneSlackBot: channel_type="slack", channel_id=route.to_channel_id(), name=f"slack-{slack_channel[:8]}", + client_type="chat", ) await self.subscribe_ws(ws_id, route.to_channel_id()) self._channel_sessions[(slack_channel, user_id)] = (ws_id, opener_ts) @@ -367,7 +386,7 @@ class TurnstoneSlackBot: text="_This session has been archived. A new one has started._", ) except Exception: - log.debug("slack.archive_session.notify_failed", ws_id=ws_id) + log.debug("slack.archive_session.notify_failed", ws_id=ws_id, exc_info=True) route = SlackRoute( channel=slack_channel, @@ -451,7 +470,10 @@ class TurnstoneSlackBot: ), ) except Exception: - log.debug("slack.notification_reply_dead_ws_notice_failed") + log.debug( + "slack.notification_reply_dead_ws_notice_failed", + exc_info=True, + ) else: log.exception("slack.notification_reply_failed") except Exception: @@ -503,6 +525,7 @@ class TurnstoneSlackBot: channel_type="slack", channel_id=route.to_channel_id(), name=f"slack-dm-{user_id[:8]}", + client_type="chat", ) if is_new: await self.subscribe_ws(ws_id, route.to_channel_id()) @@ -565,7 +588,7 @@ class TurnstoneSlackBot: blocks=[], ) except Exception: - log.debug("slack.approve_message_update_failed") + log.debug("slack.approve_message_update_failed", exc_info=True) async def _on_deny(self, ack: Any, body: dict[str, Any]) -> None: await ack() @@ -619,7 +642,7 @@ class TurnstoneSlackBot: blocks=[], ) except Exception: - log.debug("slack.deny_message_update_failed") + log.debug("slack.deny_message_update_failed", exc_info=True) async def _on_plan_approve(self, ack: Any, body: dict[str, Any]) -> None: await ack() @@ -645,7 +668,7 @@ class TurnstoneSlackBot: blocks=[], ) except Exception: - log.debug("slack.plan_review_approve_update_failed") + log.debug("slack.plan_review_approve_update_failed", exc_info=True) async def _on_plan_request_changes(self, ack: Any, body: dict[str, Any], client: Any) -> None: await ack() @@ -681,7 +704,9 @@ class TurnstoneSlackBot: }, ) - async def _on_plan_feedback_modal(self, ack: Any, body: dict[str, Any], view: dict[str, Any]) -> None: + async def _on_plan_feedback_modal( + self, ack: Any, body: dict[str, Any], view: dict[str, Any] + ) -> None: await ack() ws_id = view.get("private_metadata", "") @@ -717,7 +742,7 @@ class TurnstoneSlackBot: blocks=[], ) except Exception: - log.debug("slack.plan_review_modal_update_failed") + log.debug("slack.plan_review_modal_update_failed", exc_info=True) async def subscribe_ws(self, ws_id: str, channel_id: str) -> None: if ws_id in self._subscribed_ws: @@ -793,7 +818,7 @@ class TurnstoneSlackBot: try: data = json.loads(sse.data) except json.JSONDecodeError: - log.debug("slack.sse_invalid_json", ws_id=ws_id) + log.debug("slack.sse_invalid_json", ws_id=ws_id, exc_info=True) continue event = ServerEvent.from_dict(data) @@ -884,9 +909,7 @@ class TurnstoneSlackBot: _tool_names = [ it.get("approval_label", "") or it.get("func_name", "") for it in event.items - if it.get("needs_approval") - and it.get("func_name") - and not it.get("error") + if it.get("needs_approval") and it.get("func_name") and not it.get("error") ] _tool_names = [n for n in _tool_names if n] if _tool_names: @@ -980,16 +1003,17 @@ class TurnstoneSlackBot: text="Tool approval required", ) except Exception: - log.debug("slack.verdict_message_update_failed", ws_id=ws_id) + log.debug("slack.verdict_message_update_failed", ws_id=ws_id, exc_info=True) elif isinstance(event, PlanReviewEvent): log.info("slack.plan_review_received", ws_id=ws_id) + plan_preview = _sanitize_slack_preview(event.content, max_length=2000) blocks = [ { "type": "section", "text": { "type": "mrkdwn", - "text": f"*Plan Review*\n```{event.content[:2000]}```", + "text": f"*Plan Review*\n```{plan_preview}```", }, }, { @@ -1036,7 +1060,7 @@ class TurnstoneSlackBot: blocks=cast("list[dict[str, Any]]", []), ) except Exception: - log.debug("slack.approval_resolved_edit_failed", ws_id=ws_id) + log.debug("slack.approval_resolved_edit_failed", ws_id=ws_id, exc_info=True) elif isinstance(event, StreamEndEvent): sm = self._streaming.pop(ws_id, None) @@ -1044,10 +1068,19 @@ class TurnstoneSlackBot: if sm is not None: await sm.finalize() - reply_route = self._notify_reply_routes.get(ws_id) - if reply_route is not None and sm is not None and sm._ts: - if reply_route.channel and reply_route.user_id: - self._track_notification(sm._ts, ws_id, reply_route) + # Pop the notification-reply override so subsequent turns on + # this ws default back to the session route. Without the pop, + # one notification reply pins all future responses to the + # notification thread until the bot restarts. + reply_route = self._notify_reply_routes.pop(ws_id, None) + if ( + reply_route is not None + and sm is not None + and sm._ts + and reply_route.channel + and reply_route.user_id + ): + self._track_notification(sm._ts, ws_id, reply_route) self._pending_approval.pop(ws_id, None) @@ -1068,18 +1101,31 @@ class TurnstoneSlackBot: thread_ts: str, owner_user_id: str | None, ) -> None: - tool_lines = [] + tool_lines: list[str] = [] + body_len = 0 + truncated_items = 0 for item in items: raw_name = item.get("approval_label") or item.get("func_name") or "tool" name = raw_name.replace("&", "&").replace("<", "<").replace(">", ">") raw_preview = item.get("preview", "") - preview = _sanitize_slack_preview(raw_preview) if raw_preview else "" - - tool_lines.append( - f"• *{name}*\n```{preview}```" if preview else f"• *{name}*" + preview = ( + _sanitize_slack_preview(raw_preview, max_length=_APPROVAL_PER_ITEM_PREVIEW) + if raw_preview + else "" ) + line = f"• *{name}*\n```{preview}```" if preview else f"• *{name}*" + # +1 for the join newline + if body_len + len(line) + 1 > _APPROVAL_TEXT_BUDGET: + truncated_items = len(items) - len(tool_lines) + break + tool_lines.append(line) + body_len += len(line) + 1 + + if truncated_items > 0: + tool_lines.append(f"_…and {truncated_items} more (preview truncated)_") + blocks = [ { "type": "section", @@ -1202,4 +1248,4 @@ class TurnstoneSlackBot: self._notify_reply_routes.pop(ws_id, None) stale = [ts for ts, entry in self._notify_ws_map.items() if entry[0] == ws_id] for ts in stale: - del self._notify_ws_map[ts] \ No newline at end of file + del self._notify_ws_map[ts]