From ffc4d91fec44f017be1da673a1ac2448c8982ada Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 20:57:19 +0000 Subject: [PATCH 1/7] =?UTF-8?q?feat(messenger):=20adapter=20=E2=80=94=20we?= =?UTF-8?q?bhook,=20Graph=20API,=20send/stream=20(vercel/chat#461)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port of `packages/adapter-messenger/src/index.ts` (PR 2 of 2). Builds on the scaffolding (types/format converter/cards) added in PR #118. Includes: - `MessengerAdapter`: webhook routing (GET verification + POST events), X-Hub-Signature-256 HMAC-SHA256 verification, Graph API client backed by aiohttp, send paths (text / Generic template / Button template / text fallback), buffered streaming, postback / reaction / echo / delivery / read handling, attachment extraction with lazy download, message cache (Messenger has no history API), user-profile cache, thread / channel helpers, typed Graph-API error mapping. - `create_messenger_adapter` factory with FACEBOOK_* env fallbacks. - Exports wired through `chat_sdk.adapters.messenger`. Q1 (init-failure behavior, see #110): match `WhatsAppAdapter` — raise `ValidationError` from both the factory and the constructor when any required credential is missing, so config errors surface loudly at startup. Q3 (signature verification, see #110): pin upstream's X-Hub-Signature-256 + App Secret HMAC contract. A swappable verifier (Slack-style) would diverge from Meta's protocol with no offsetting benefit for a single-secret integration; flagged as a possible future divergence but not introduced here. Tests: `tests/test_messenger_webhook.py` (57) and `tests/test_messenger_api.py` (64) — mirrors the Telegram/WhatsApp file split. Covers signature valid/invalid/missing/wrong-algo/replay, webhook routing for all event types, postback decoding (raw + chat: prefix), card → template paths, stream buffering, truncation, error mapping, and the Q1 constructor failure path. 121 new tests, full suite: 4220 passed, 3 skipped. --- src/chat_sdk/adapters/messenger/__init__.py | 18 +- src/chat_sdk/adapters/messenger/adapter.py | 1127 +++++++++++++++++++ tests/test_messenger_api.py | 890 +++++++++++++++ tests/test_messenger_webhook.py | 851 ++++++++++++++ 4 files changed, 2883 insertions(+), 3 deletions(-) create mode 100644 src/chat_sdk/adapters/messenger/adapter.py create mode 100644 tests/test_messenger_api.py create mode 100644 tests/test_messenger_webhook.py diff --git a/src/chat_sdk/adapters/messenger/__init__.py b/src/chat_sdk/adapters/messenger/__init__.py index 0e86386..11d9d6b 100644 --- a/src/chat_sdk/adapters/messenger/__init__.py +++ b/src/chat_sdk/adapters/messenger/__init__.py @@ -1,10 +1,17 @@ """Messenger (Meta) adapter for chat-sdk. -PR 1 of 2 (scaffolding): types, format converter, and card conversion. -The adapter itself (webhook routing, Graph API, signature verification, -send/stream) is added in PR 2. +Python port of upstream ``packages/adapter-messenger``. Supports webhook +routing (with HMAC-SHA256 signature verification), Send API integration via +the Meta Graph API, and message/card/streaming primitives. """ +from chat_sdk.adapters.messenger.adapter import ( + DEFAULT_API_VERSION, + GRAPH_API_BASE, + MESSENGER_MESSAGE_LIMIT, + MessengerAdapter, + create_messenger_adapter, +) from chat_sdk.adapters.messenger.cards import ( MessengerCardResult, card_to_messenger, @@ -30,6 +37,10 @@ ) __all__ = [ + "DEFAULT_API_VERSION", + "GRAPH_API_BASE", + "MESSENGER_MESSAGE_LIMIT", + "MessengerAdapter", "MessengerAdapterConfig", "MessengerButton", "MessengerButtonTemplatePayload", @@ -47,6 +58,7 @@ "MessengerWebhookPayload", "card_to_messenger", "card_to_messenger_text", + "create_messenger_adapter", "decode_messenger_callback_data", "encode_messenger_callback_data", ] diff --git a/src/chat_sdk/adapters/messenger/adapter.py b/src/chat_sdk/adapters/messenger/adapter.py new file mode 100644 index 0000000..de151df --- /dev/null +++ b/src/chat_sdk/adapters/messenger/adapter.py @@ -0,0 +1,1127 @@ +"""Messenger (Meta) adapter for chat SDK. + +Supports messaging via the Meta Messenger Platform (Graph API). +All conversations are 1:1 DMs between the Page and a user (PSID). + +Python port of ``packages/adapter-messenger/src/index.ts`` (PR 2 of 2 of +the Messenger port; PR 1 added types, format converter, and cards). + +See: https://developers.facebook.com/docs/messenger-platform +""" + +from __future__ import annotations + +import hashlib +import hmac +import inspect +import json +import os +import time +from collections.abc import AsyncIterable +from datetime import datetime, timezone +from typing import Any, cast +from urllib.parse import parse_qs, urlparse + +from chat_sdk.adapters.messenger.cards import ( + MessengerCardResultTemplate, + MessengerCardResultText, + card_to_messenger, + decode_messenger_callback_data, +) +from chat_sdk.adapters.messenger.format_converter import MessengerFormatConverter +from chat_sdk.adapters.messenger.types import ( + ENV_APP_SECRET, + ENV_PAGE_ACCESS_TOKEN, + ENV_VERIFY_TOKEN, + MessengerAdapterConfig, + MessengerMessagingEvent, + MessengerRawMessage, + MessengerSendApiResponse, + MessengerTemplatePayload, + MessengerThreadId, + MessengerUserProfile, + MessengerWebhookPayload, +) +from chat_sdk.emoji import convert_emoji_placeholders, default_emoji_resolver +from chat_sdk.logger import ConsoleLogger, Logger +from chat_sdk.shared.adapter_utils import extract_card +from chat_sdk.shared.errors import ( + AdapterRateLimitError, + AuthenticationError, + NetworkError, + ResourceNotFoundError, + ValidationError, +) +from chat_sdk.types import ( + ActionEvent, + AdapterPostableMessage, + Attachment, + Author, + ChannelInfo, + ChatInstance, + EmojiValue, + FetchOptions, + FetchResult, + FormattedContent, + LockScope, + Message, + MessageMetadata, + PostableMarkdown, + RawMessage, + ReactionEvent, + StreamChunk, + StreamOptions, + ThreadInfo, + WebhookOptions, +) + +# Meta Graph API base URL +GRAPH_API_BASE = "https://graph.facebook.com" +# Default Graph API version (matches upstream) +DEFAULT_API_VERSION = "v21.0" +# Maximum text length the Send API will accept in a single message +MESSENGER_MESSAGE_LIMIT = 2000 +# Suffix that encodes a sequence number on Send-API mids (``mid.abc:2`` etc.) +# Used to disambiguate identical-timestamp messages in the local cache. +_MESSAGE_SEQUENCE_SUFFIX = ":" + + +class MessengerAdapter: + """Messenger (Meta) adapter for chat SDK. + + Implements the chat-sdk ``Adapter`` interface for the Messenger Platform. + """ + + def __init__(self, config: MessengerAdapterConfig) -> None: + # Upstream's constructor takes the resolved credentials directly. Our + # ``MessengerAdapterConfig`` exposes them via ``resolved_*`` helpers so + # the constructor and the factory both go through the same fallback + # chain. Constructing the adapter without resolved credentials is a + # programmer error — the factory enforces presence, and direct callers + # opt in to env-var fallbacks by leaving fields as ``None``. + app_secret = config.resolved_app_secret() + page_access_token = config.resolved_page_access_token() + verify_token = config.resolved_verify_token() + if not app_secret: + raise ValidationError( + "messenger", + f"appSecret is required. Set {ENV_APP_SECRET} or provide it in config.", + ) + if not page_access_token: + raise ValidationError( + "messenger", + f"pageAccessToken is required. Set {ENV_PAGE_ACCESS_TOKEN} or provide it in config.", + ) + if not verify_token: + raise ValidationError( + "messenger", + f"verifyToken is required. Set {ENV_VERIFY_TOKEN} or provide it in config.", + ) + + self._name = "messenger" + self._lock_scope: LockScope = "channel" + self._persist_message_history = True + + self._app_secret = app_secret + self._page_access_token = page_access_token + self._verify_token = verify_token + self._api_version = config.api_version or DEFAULT_API_VERSION + self._graph_api_url = f"{GRAPH_API_BASE}/{self._api_version}" + self._logger: Logger = config.logger + self._format_converter = MessengerFormatConverter() + + # If a user_name is provided we treat it as explicit and never + # overwrite it from ``/me`` / ``chat.get_user_name()`` — matches the + # upstream ``hasExplicitUserName`` gate. + self._has_explicit_user_name = bool(config.user_name) + self._user_name = config.user_name or "bot" + + self._chat: ChatInstance | None = None + self._bot_user_id: str | None = None + + # Local caches. Messenger has no message-history API, so the adapter + # holds every parsed message in memory to back ``fetch_messages``. + self._message_cache: dict[str, list[Message]] = {} + self._user_profile_cache: dict[str, MessengerUserProfile] = {} + + # Shared aiohttp session for connection pooling; created lazily. + self._http_session: Any | None = None + + # ========================================================================= + # Adapter interface properties + # ========================================================================= + + @property + def name(self) -> str: + return self._name + + @property + def lock_scope(self) -> LockScope: + return self._lock_scope + + @property + def persist_message_history(self) -> bool: + return self._persist_message_history + + @property + def user_name(self) -> str: + return self._user_name + + @property + def bot_user_id(self) -> str | None: + return self._bot_user_id + + # ========================================================================= + # Lifecycle + # ========================================================================= + + async def initialize(self, chat: ChatInstance) -> None: + """Initialize the adapter and fetch Page identity (best-effort).""" + self._chat = chat + + if not self._has_explicit_user_name: + self._user_name = chat.get_user_name() + + try: + me = await self._graph_api_fetch("me", method="GET") + self._bot_user_id = cast(str, me.get("id")) + name = me.get("name") + if not self._has_explicit_user_name and name: + self._user_name = name + self._logger.info( + "Messenger adapter initialized", + {"botUserId": self._bot_user_id, "userName": self._user_name}, + ) + except Exception as error: + # Match upstream: identity fetch failure is non-fatal; the + # adapter still functions for incoming webhook events. + self._logger.warn( + "Failed to fetch Messenger page identity", + {"error": str(error)}, + ) + + async def disconnect(self) -> None: + """Cleanup hook. Close the shared HTTP session if it was created.""" + if self._http_session is not None and not getattr(self._http_session, "closed", True): + await self._http_session.close() + self._http_session = None + + async def _get_http_session(self) -> Any: + """Return the shared aiohttp session, creating it lazily on first use.""" + import aiohttp + + if self._http_session is None or getattr(self._http_session, "closed", True): + self._http_session = aiohttp.ClientSession() + return self._http_session + + # ========================================================================= + # Webhook + # ========================================================================= + + async def handle_webhook( + self, + request: Any, + options: WebhookOptions | None = None, + ) -> Any: + """Handle an incoming webhook request. + + - GET: webhook subscription verification challenge. + - POST: event notifications (messages, postbacks, reactions, ...). + """ + method = getattr(request, "method", "POST") + if method == "GET": + return self._handle_verification(request) + + body = await self._get_request_body(request) + + signature = self._get_header(request, "x-hub-signature-256") + if not self._verify_signature(body, signature): + self._logger.warn("Messenger webhook rejected due to invalid signature") + return self._make_response("Invalid signature", 403) + + try: + payload: MessengerWebhookPayload = json.loads(body) + except (json.JSONDecodeError, ValueError): + return self._make_response("Invalid JSON", 400) + + if payload.get("object") != "page": + return self._make_response("Not a page subscription", 404) + + if not self._chat: + self._logger.warn("Chat instance not initialized, ignoring Messenger webhook") + return self._make_response("EVENT_RECEIVED", 200) + + for entry in payload.get("entry", []): + for event in entry.get("messaging", []): + message = event.get("message") + if message and not message.get("is_echo"): + self._handle_incoming_message(event, options) + elif message and message.get("is_echo"): + self._handle_echo(event) + + if event.get("postback"): + self._handle_postback(event, options) + + if event.get("reaction"): + self._handle_reaction(event, options) + + delivery = event.get("delivery") + if delivery: + self._logger.debug( + "Message delivery confirmation", + { + "watermark": delivery.get("watermark"), + "mids": delivery.get("mids"), + }, + ) + + read = event.get("read") + if read: + self._logger.debug( + "Message read confirmation", + {"watermark": read.get("watermark")}, + ) + + return self._make_response("EVENT_RECEIVED", 200) + + def _handle_verification(self, request: Any) -> Any: + """Handle the GET subscription challenge.""" + url = getattr(request, "url", "") + parsed = urlparse(url) + params = parse_qs(parsed.query) + + mode = (params.get("hub.mode") or [None])[0] + token = (params.get("hub.verify_token") or [None])[0] + challenge = (params.get("hub.challenge") or [""])[0] + + if mode == "subscribe" and token == self._verify_token: + self._logger.info("Messenger webhook verified") + return self._make_response(challenge, 200) + + self._logger.warn("Messenger webhook verification failed") + return self._make_response("Forbidden", 403) + + def _verify_signature(self, body: str, signature: str | None) -> bool: + """Verify the ``X-Hub-Signature-256`` header (HMAC-SHA256, App Secret). + + Format: ``sha256=``. Returns ``False`` on any malformed input. + Q3 (see #110): we keep upstream's hard-wired ``app_secret``-based + HMAC scheme. A swappable verifier (the pattern used by the Slack + adapter) would diverge from upstream's contract and isn't justified + for a single-secret Meta integration; flagged as a possible future + divergence but not introduced here. + """ + if not signature: + return False + + # Header is ``algo=hash``. Reject anything that isn't sha256 hex. + parts = signature.split("=", 1) + if len(parts) != 2: + return False + algo, hash_hex = parts + if algo != "sha256" or not hash_hex: + return False + + try: + computed_hex = hmac.new( + self._app_secret.encode("utf-8"), + body.encode("utf-8"), + hashlib.sha256, + ).hexdigest() + # Compare hex strings directly. The signature is presented as a + # lowercase hex string by Meta; ``hexdigest()`` is lowercase. + return hmac.compare_digest(hash_hex, computed_hex) + except Exception: + self._logger.warn("Failed to verify Messenger webhook signature") + return False + + # ========================================================================= + # Event handlers + # ========================================================================= + + def _handle_incoming_message( + self, + event: MessengerMessagingEvent, + options: WebhookOptions | None = None, + ) -> None: + if not self._chat: + return + + sender = event.get("sender") or {} + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=sender.get("id", ""))) + + parsed = self._parse_messenger_message(event, thread_id) + self._cache_message(parsed) + self._chat.process_message(self, thread_id, parsed, options) + + def _handle_echo(self, event: MessengerMessagingEvent) -> None: + """Cache echoed (bot-sent) messages but don't dispatch them.""" + if not event.get("message"): + return + + recipient = event.get("recipient") or {} + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=recipient.get("id", ""))) + + parsed = self._parse_messenger_message(event, thread_id) + self._cache_message(parsed) + + def _handle_postback( + self, + event: MessengerMessagingEvent, + options: WebhookOptions | None = None, + ) -> None: + postback = event.get("postback") + if not (self._chat and postback): + return + + sender = event.get("sender") or {} + sender_id = sender.get("id", "") + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=sender_id)) + + decoded = decode_messenger_callback_data(postback.get("payload")) + action_id = decoded.get("action_id") or "" + value = decoded.get("value") + + message_id = postback.get("mid") or f"postback:{event.get('timestamp', 0)}" + + self._chat.process_action( + ActionEvent( + adapter=self, + thread=None, # filled in by Chat + thread_id=thread_id, + message_id=message_id, + user=Author( + user_id=sender_id, + user_name=sender_id, + full_name=sender_id, + is_bot=False, + is_me=False, + ), + action_id=action_id, + value=value, + raw=event, + ), + options, + ) + + def _handle_reaction( + self, + event: MessengerMessagingEvent, + options: WebhookOptions | None = None, + ) -> None: + reaction = event.get("reaction") + if not (self._chat and reaction): + return + + sender = event.get("sender") or {} + sender_id = sender.get("id", "") + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=sender_id)) + + added = reaction.get("action") == "react" + raw_emoji = reaction.get("emoji", "") + # Resolve to an EmojiValue using the gchat-style raw unicode resolver, + # mirroring upstream's ``defaultEmojiResolver.fromGChat(...)`` call. + emoji_value = default_emoji_resolver.from_gchat(raw_emoji) + + self._chat.process_reaction( + ReactionEvent( + adapter=self, + thread=None, # pyrefly: ignore[bad-argument-type] # filled in by Chat + thread_id=thread_id, + message_id=reaction.get("mid", ""), + user=Author( + user_id=sender_id, + user_name=sender_id, + full_name=sender_id, + is_bot=False, + is_me=False, + ), + emoji=emoji_value, + raw_emoji=raw_emoji, + added=added, + raw=event, + ), + options, + ) + + # ========================================================================= + # Sending messages + # ========================================================================= + + async def post_message( + self, + thread_id: str, + message: AdapterPostableMessage, + ) -> RawMessage: + """Send a message to a Messenger user.""" + card = extract_card(message) + if card: + result = card_to_messenger(card) + if result.get("type") == "template": + template_payload = cast(MessengerCardResultTemplate, result)["payload"] + # Convert emoji placeholders inside the JSON-encoded payload + # to mirror upstream's ``convertEmojiPlaceholders`` pass. + converted = cast( + MessengerTemplatePayload, + json.loads( + convert_emoji_placeholders( + json.dumps(template_payload), + "messenger", + ) + ), + ) + return await self._send_template_message(thread_id, converted) + # Text fallback + return await self._send_text_message( + thread_id, + convert_emoji_placeholders( + cast(MessengerCardResultText, result)["text"], + "messenger", + ), + ) + + # Regular text/markdown/AST message + text = convert_emoji_placeholders( + self._format_converter.render_postable(message), + "messenger", + ) + return await self._send_text_message(thread_id, text) + + async def _send_text_message(self, thread_id: str, text: str) -> RawMessage: + """Send a plain text message via the Send API.""" + recipient_id = self._resolve_thread_id(thread_id).recipient_id + truncated = self._truncate_message(text) + + if not truncated.strip(): + raise ValidationError("messenger", "Message text cannot be empty") + + result: MessengerSendApiResponse = await self._graph_api_fetch( + "me/messages", + method="POST", + body={ + "recipient": {"id": recipient_id}, + "message": {"text": truncated}, + "messaging_type": "RESPONSE", + }, + ) + + raw_event: MessengerMessagingEvent = { + "sender": {"id": self._bot_user_id or ""}, + "recipient": {"id": recipient_id}, + "timestamp": int(time.time() * 1000), + "message": { + "mid": result["message_id"], + "text": truncated, + "is_echo": True, + }, + } + + parsed = self._parse_messenger_message(raw_event, thread_id) + self._cache_message(parsed) + + return RawMessage( + id=result["message_id"], + thread_id=thread_id, + raw=raw_event, + ) + + async def _send_template_message( + self, + thread_id: str, + payload: MessengerTemplatePayload, + ) -> RawMessage: + """Send a Generic / Button template message via the Send API.""" + recipient_id = self._resolve_thread_id(thread_id).recipient_id + + result: MessengerSendApiResponse = await self._graph_api_fetch( + "me/messages", + method="POST", + body={ + "recipient": {"id": recipient_id}, + "message": { + "attachment": { + "type": "template", + "payload": payload, + }, + }, + "messaging_type": "RESPONSE", + }, + ) + + raw_event: MessengerMessagingEvent = { + "sender": {"id": self._bot_user_id or ""}, + "recipient": {"id": recipient_id}, + "timestamp": int(time.time() * 1000), + "message": { + "mid": result["message_id"], + "is_echo": True, + }, + } + + parsed = self._parse_messenger_message(raw_event, thread_id) + self._cache_message(parsed) + + return RawMessage( + id=result["message_id"], + thread_id=thread_id, + raw=raw_event, + ) + + async def edit_message( + self, + thread_id: str, + message_id: str, + message: AdapterPostableMessage, + ) -> RawMessage: + """Messenger Send API does not support editing — raises.""" + raise ValidationError("messenger", "Messenger does not support editing messages") + + async def stream( + self, + thread_id: str, + text_stream: AsyncIterable[str | StreamChunk], + options: StreamOptions | None = None, + ) -> RawMessage: + """Buffer all stream chunks and send as a single message. + + Messenger doesn't support message edits, so we can't incrementally + update the same message. Mirrors upstream's behavior exactly. + """ + accumulated = "" + async for chunk in text_stream: + if isinstance(chunk, str): + accumulated += chunk + elif getattr(chunk, "type", None) == "markdown_text": + accumulated += getattr(chunk, "text", "") + return await self.post_message(thread_id, PostableMarkdown(markdown=accumulated)) + + async def delete_message(self, thread_id: str, message_id: str) -> None: + """Messenger Send API does not support deleting — raises.""" + raise ValidationError("messenger", "Messenger does not support deleting messages") + + async def add_reaction( + self, + thread_id: str, + message_id: str, + emoji: EmojiValue | str, + ) -> None: + """Messenger Send API does not expose reaction send — raises.""" + raise ValidationError("messenger", "Messenger does not support reactions via API") + + async def remove_reaction( + self, + thread_id: str, + message_id: str, + emoji: EmojiValue | str, + ) -> None: + """Messenger Send API does not expose reaction send — raises.""" + raise ValidationError("messenger", "Messenger does not support reactions via API") + + async def start_typing(self, thread_id: str, status: str | None = None) -> None: + """Send a ``typing_on`` sender_action via the Send API.""" + recipient_id = self._resolve_thread_id(thread_id).recipient_id + await self._graph_api_fetch( + "me/messages", + method="POST", + body={ + "recipient": {"id": recipient_id}, + "sender_action": "typing_on", + }, + ) + + # ========================================================================= + # Fetching + # ========================================================================= + + async def fetch_messages( + self, + thread_id: str, + options: FetchOptions | None = None, + ) -> FetchResult: + """Fetch messages from the local cache (Messenger has no history API).""" + opts = options or FetchOptions() + messages = list(self._message_cache.get(thread_id, [])) + messages.sort(key=self._sort_key) + return self._paginate_messages(messages, opts) + + async def fetch_message( + self, + thread_id: str, + message_id: str, + ) -> Message | None: + """Fetch a single cached message by ID across all thread caches.""" + for messages in self._message_cache.values(): + for msg in messages: + if msg.id == message_id: + return msg + return None + + async def fetch_thread(self, thread_id: str) -> ThreadInfo: + """Fetch thread info, hydrated with the user profile when available.""" + recipient_id = self._resolve_thread_id(thread_id).recipient_id + profile = await self._fetch_user_profile(recipient_id) + display_name = self._profile_display_name(profile) + + # On Messenger every conversation is a 1:1 DM, so channel == thread. + return ThreadInfo( + id=thread_id, + channel_id=thread_id, + channel_name=display_name, + is_dm=True, + metadata={"profile": dict(profile)}, + ) + + async def fetch_channel_info(self, channel_id: str) -> ChannelInfo: + """Fetch channel info (channel == thread on Messenger).""" + recipient_id = self._resolve_thread_id(channel_id).recipient_id + profile = await self._fetch_user_profile(recipient_id) + display_name = self._profile_display_name(profile) + + return ChannelInfo( + id=channel_id, + name=display_name, + is_dm=True, + metadata={"profile": dict(profile)}, + ) + + # ========================================================================= + # Thread ID encoding + # ========================================================================= + + def channel_id_from_thread_id(self, thread_id: str) -> str: + """On Messenger every conversation is a 1:1 DM, channel == thread.""" + return thread_id + + def is_dm(self, thread_id: str) -> bool: + """All Messenger conversations are DMs.""" + return True + + async def open_dm(self, user_id: str) -> str: + """Open a DM with a user. Returns the encoded thread ID.""" + return self.encode_thread_id(MessengerThreadId(recipient_id=user_id)) + + def encode_thread_id(self, platform_data: MessengerThreadId) -> str: + """Encode a Messenger thread ID. Format: ``messenger:{recipientId}``.""" + return f"messenger:{platform_data.recipient_id}" + + def decode_thread_id(self, thread_id: str) -> MessengerThreadId: + """Decode a Messenger thread ID. Format: ``messenger:{recipientId}``.""" + parts = thread_id.split(":") + if parts[0] != "messenger" or len(parts) != 2: + raise ValidationError("messenger", f"Invalid Messenger thread ID: {thread_id}") + recipient_id = parts[1] + if not recipient_id: + raise ValidationError("messenger", f"Invalid Messenger thread ID: {thread_id}") + return MessengerThreadId(recipient_id=recipient_id) + + def _resolve_thread_id(self, value: str) -> MessengerThreadId: + """Resolve a value to a ``MessengerThreadId``. + + Accepts both encoded thread IDs (``messenger:PSID``) and bare PSIDs, + mirroring upstream's ``resolveThreadId`` helper. + """ + if value.startswith("messenger:"): + return self.decode_thread_id(value) + return MessengerThreadId(recipient_id=value) + + # ========================================================================= + # Parsing + # ========================================================================= + + def parse_message(self, raw: MessengerRawMessage) -> Message: + """Parse a raw messaging event into a normalized ``Message``.""" + sender = raw.get("sender") or {} + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=sender.get("id", ""))) + message = self._parse_messenger_message(raw, thread_id) + self._cache_message(message) + return message + + def render_formatted(self, content: FormattedContent) -> str: + """Render formatted AST content back to Messenger text.""" + return self._format_converter.from_ast(content) + + def _parse_messenger_message( + self, + event: MessengerMessagingEvent, + thread_id: str, + ) -> Message: + message = event.get("message") or {} + postback = event.get("postback") or {} + sender = event.get("sender") or {} + text = message.get("text") or postback.get("title") or "" + is_echo = bool(message.get("is_echo")) + is_me = is_echo or (sender.get("id") == self._bot_user_id and self._bot_user_id is not None) + + mid = message.get("mid") + timestamp = event.get("timestamp", 0) + msg_id = mid if mid else f"event:{timestamp}" + + return Message( + id=msg_id, + thread_id=thread_id, + text=text, + formatted=self._format_converter.to_ast(text), + raw=event, + author=Author( + user_id=sender.get("id", ""), + user_name=sender.get("id", ""), + full_name=sender.get("id", ""), + is_bot=is_me, + is_me=is_me, + ), + metadata=MessageMetadata( + date_sent=datetime.fromtimestamp(timestamp / 1000.0, tz=timezone.utc), + edited=False, + ), + attachments=self._extract_attachments(event), + is_mention=True, + ) + + def _extract_attachments(self, event: MessengerMessagingEvent) -> list[Attachment]: + message = event.get("message") or {} + attachments = message.get("attachments") or [] + result: list[Attachment] = [] + for attachment in attachments: + payload = attachment.get("payload") or {} + url = payload.get("url") + if not url: + continue + result.append( + Attachment( + type=self._map_attachment_type(attachment.get("type", "")), + url=url, + fetch_data=self._make_attachment_downloader(url), + ) + ) + return result + + def _make_attachment_downloader(self, url: str) -> Any: + """Build a closure that downloads ``url`` lazily. + + Kept as a named helper so the ``url`` capture isn't subject to + late-binding when multiple attachments are extracted from a single + event. + """ + + async def _download() -> bytes: + return await self._download_attachment(url) + + return _download + + @staticmethod + def _map_attachment_type(fb_type: str) -> str: + """Map a Messenger attachment type to the SDK ``Attachment.type`` enum.""" + if fb_type == "image": + return "image" + if fb_type == "video": + return "video" + if fb_type == "audio": + return "audio" + return "file" + + async def _download_attachment(self, url: str) -> bytes: + """Download an attachment payload URL. Wraps errors in ``NetworkError``.""" + try: + session = await self._get_http_session() + async with session.get(url) as response: + if response.status != 200: + raise NetworkError( + "messenger", + f"Failed to download Messenger attachment: {response.status}", + ) + return await response.read() + except NetworkError: + raise + except Exception as error: + raise NetworkError( + "messenger", + "Failed to download Messenger attachment", + original_error=error if isinstance(error, Exception) else None, + ) from error + + # ========================================================================= + # User profile (with cache) + # ========================================================================= + + async def _fetch_user_profile(self, user_id: str) -> MessengerUserProfile: + cached = self._user_profile_cache.get(user_id) + if cached: + return cached + + try: + profile = await self._graph_api_fetch( + user_id, + method="GET", + query_params={"fields": "first_name,last_name,profile_pic"}, + ) + self._user_profile_cache[user_id] = profile + return cast(MessengerUserProfile, profile) + except Exception: + # On any error, fall back to a minimal profile carrying just the + # user ID. Matches upstream's silent fallback. + return {"id": user_id} + + @staticmethod + def _profile_display_name(profile: MessengerUserProfile) -> str: + parts = [p for p in (profile.get("first_name"), profile.get("last_name")) if p] + if parts: + return " ".join(parts) + return profile.get("id", "") + + # ========================================================================= + # Cache / pagination + # ========================================================================= + + def _cache_message(self, message: Message) -> None: + existing = self._message_cache.get(message.thread_id, []) + for i, item in enumerate(existing): + if item.id == message.id: + existing[i] = message + break + else: + existing.append(message) + existing.sort(key=self._sort_key) + self._message_cache[message.thread_id] = existing + + @staticmethod + def _sort_key(message: Message) -> tuple[float, int]: + """Sort by ``(date_sent, sequence_suffix)``. + + Messages with the same timestamp but a ``:N`` sequence suffix on the + ID are ordered by N. Mirrors upstream's ``compareMessages``. + """ + date_value = message.metadata.date_sent.timestamp() if message.metadata else 0.0 + seq = 0 + if _MESSAGE_SEQUENCE_SUFFIX in message.id: + tail = message.id.rsplit(_MESSAGE_SEQUENCE_SUFFIX, 1)[1] + if tail.isdigit(): + seq = int(tail) + return (date_value, seq) + + @staticmethod + def _paginate_messages(messages: list[Message], options: FetchOptions) -> FetchResult: + limit = max(1, min(options.limit or 50, 100)) + direction = options.direction or "backward" + + if not messages: + return FetchResult(messages=[]) + + index_by_id = {m.id: i for i, m in enumerate(messages)} + + if direction == "backward": + end = index_by_id[options.cursor] if options.cursor and options.cursor in index_by_id else len(messages) + start = max(0, end - limit) + page = messages[start:end] + return FetchResult( + messages=page, + next_cursor=(page[0].id if start > 0 and page else None), + ) + + # forward + start = index_by_id[options.cursor] + 1 if options.cursor and options.cursor in index_by_id else 0 + end = min(len(messages), start + limit) + page = messages[start:end] + return FetchResult( + messages=page, + next_cursor=(page[-1].id if end < len(messages) and page else None), + ) + + # ========================================================================= + # Send-API helpers + # ========================================================================= + + @staticmethod + def _truncate_message(text: str) -> str: + if len(text) <= MESSENGER_MESSAGE_LIMIT: + return text + return f"{text[: MESSENGER_MESSAGE_LIMIT - 3]}..." + + async def _graph_api_fetch( + self, + endpoint: str, + *, + method: str, + body: dict[str, Any] | None = None, + query_params: dict[str, str] | None = None, + ) -> Any: + """Call the Meta Graph API with the page access token. + + Wraps the call in standardized adapter errors. Mirrors the upstream + ``graphApiFetch`` helper's surface (URL building, query params, JSON + parse-then-status check, error-code mapping). + """ + session = await self._get_http_session() + url = f"{self._graph_api_url}/{endpoint}" + params: dict[str, str] = {"access_token": self._page_access_token} + if query_params: + params.update(query_params) + + try: + if method == "GET": + response_ctx = session.get(url, params=params) + else: + response_ctx = session.post( + url, + params=params, + headers={"Content-Type": "application/json"}, + json=body, + ) + + async with response_ctx as response: + status = response.status + try: + data = await response.json(content_type=None) + except Exception as error: + raise NetworkError( + "messenger", + f"Failed to parse Messenger API response for {endpoint}", + ) from error + + if status < 200 or status >= 300: + self._throw_graph_api_error(endpoint, status, data or {}) + + return data + except NetworkError: + raise + except AdapterRateLimitError: + raise + except AuthenticationError: + raise + except ResourceNotFoundError: + raise + except ValidationError: + raise + except Exception as error: + raise NetworkError( + "messenger", + f"Network error calling Messenger Graph API {endpoint}", + original_error=error if isinstance(error, Exception) else None, + ) from error + + @staticmethod + def _throw_graph_api_error( + endpoint: str, + status: int, + data: dict[str, Any], + ) -> None: + """Translate a non-2xx Graph API response to a typed adapter error.""" + error = data.get("error") if isinstance(data, dict) else None + if not isinstance(error, dict): + error = {} + message = error.get("message") or f"Messenger API {endpoint} failed" + code = error.get("code") if error.get("code") is not None else status + + # Rate limiting: HTTP 429 or known Meta rate-limit codes. + if status == 429 or code in (4, 32, 613): + raise AdapterRateLimitError("messenger") + # Auth: HTTP 401 or Meta auth code 190. + if status == 401 or code == 190: + raise AuthenticationError("messenger", message) + # Permission: HTTP 403 or Meta permission codes 10 / 200. + if status == 403 or code in (10, 200): + raise ValidationError("messenger", message) + # Not found: HTTP 404. + if status == 404: + raise ResourceNotFoundError("messenger", endpoint) + raise NetworkError( + "messenger", + f"{message} (status {status}, code {code})", + ) + + # ========================================================================= + # Request helpers (framework-agnostic) + # ========================================================================= + + @staticmethod + async def _get_request_body(request: Any) -> str: + """Extract body text from a request object (sync or async).""" + text_attr = getattr(request, "text", None) + if text_attr is not None: + if callable(text_attr): + result = text_attr() + text_attr = await result if inspect.isawaitable(result) else result + return text_attr.decode("utf-8") if isinstance(text_attr, (bytes, bytearray)) else str(text_attr) + body = getattr(request, "body", None) + if body is not None: + if callable(body): + result = body() + body = await result if inspect.isawaitable(result) else result + if isinstance(body, (bytes, bytearray)): + return body.decode("utf-8") + return str(body) + return "" + + @staticmethod + def _get_header(request: Any, name: str) -> str | None: + """Get a header value from a request object (case-insensitive).""" + if hasattr(request, "headers"): + headers = request.headers + if isinstance(headers, dict): + for k, v in headers.items(): + if k.lower() == name.lower(): + return v + return None + return headers.get(name) + return None + + @staticmethod + def _make_response(body: str, status: int) -> dict[str, Any]: + """Create a framework-agnostic response dict.""" + return {"body": body, "status": status} + + +# ============================================================================= +# Factory +# ============================================================================= + + +def create_messenger_adapter( + *, + app_secret: str | None = None, + page_access_token: str | None = None, + verify_token: str | None = None, + api_version: str | None = None, + logger: Logger | None = None, + user_name: str | None = None, +) -> MessengerAdapter: + """Factory for ``MessengerAdapter`` with env-var fallbacks. + + Q1 (see #110): init-failure behavior. We match the upstream contract and + the sibling ``WhatsAppAdapter`` by raising ``ValidationError`` at + construction time for each missing required credential. This keeps the + Meta-family adapters consistent and surfaces config errors loudly during + startup rather than at first webhook call. + """ + _logger = logger or ConsoleLogger("info").child("messenger") + + _app_secret = app_secret or os.environ.get(ENV_APP_SECRET) + if not _app_secret: + raise ValidationError( + "messenger", + f"appSecret is required. Set {ENV_APP_SECRET} or provide it in config.", + ) + + _page_access_token = page_access_token or os.environ.get(ENV_PAGE_ACCESS_TOKEN) + if not _page_access_token: + raise ValidationError( + "messenger", + f"pageAccessToken is required. Set {ENV_PAGE_ACCESS_TOKEN} or provide it in config.", + ) + + _verify_token = verify_token or os.environ.get(ENV_VERIFY_TOKEN) + if not _verify_token: + raise ValidationError( + "messenger", + f"verifyToken is required. Set {ENV_VERIFY_TOKEN} or provide it in config.", + ) + + return MessengerAdapter( + MessengerAdapterConfig( + app_secret=_app_secret, + page_access_token=_page_access_token, + verify_token=_verify_token, + api_version=api_version, + logger=_logger, + user_name=user_name, + ) + ) diff --git a/tests/test_messenger_api.py b/tests/test_messenger_api.py new file mode 100644 index 0000000..f29f63b --- /dev/null +++ b/tests/test_messenger_api.py @@ -0,0 +1,890 @@ +"""Tests for the Messenger adapter — Graph API send & stream paths. + +Covers ``post_message`` (text / markdown / AST / generic-template card / +button-template card / text fallback), ``stream`` accumulation, +``start_typing``, ``edit_message`` / ``delete_message`` / +``add_reaction`` / ``remove_reaction`` unsupported paths, message +truncation, message caching after send, and Graph API error mapping +(rate limit / auth / not-found / network). + +Stubs the adapter's private ``_graph_api_fetch`` helper so we never hit +the network, mirroring the WhatsApp ``test_whatsapp_api.py`` pattern. + +Pairs with ``tests/test_messenger_webhook.py``. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from chat_sdk.adapters.messenger.adapter import ( + MESSENGER_MESSAGE_LIMIT, + MessengerAdapter, +) +from chat_sdk.adapters.messenger.types import MessengerAdapterConfig +from chat_sdk.logger import ConsoleLogger +from chat_sdk.shared.errors import ( + AdapterRateLimitError, + AuthenticationError, + NetworkError, + ResourceNotFoundError, + ValidationError, +) +from chat_sdk.types import MarkdownTextChunk, StreamChunk + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +THREAD_ID = "messenger:USER_123" +RECIPIENT_ID = "USER_123" + + +def _make_adapter(**overrides: Any) -> MessengerAdapter: + defaults: dict[str, Any] = { + "app_secret": "test-app-secret", + "page_access_token": "test-page-token", + "verify_token": "test-verify-token", + "user_name": "test-bot", + "logger": ConsoleLogger("error"), + } + defaults.update(overrides) + return MessengerAdapter(MessengerAdapterConfig(**defaults)) + + +def _send_api_response(message_id: str = "mid.sent") -> dict[str, Any]: + return {"recipient_id": RECIPIENT_ID, "message_id": message_id} + + +# --------------------------------------------------------------------------- +# Lifecycle +# --------------------------------------------------------------------------- + + +class TestInitialize: + """``initialize`` fetches Page identity, but failure is non-fatal.""" + + @pytest.mark.asyncio + async def test_initialize_sets_bot_id_and_name(self) -> None: + adapter = _make_adapter() + # Drop explicit user_name so /me name is used. + adapter._has_explicit_user_name = False + adapter._user_name = "bot" + adapter._graph_api_fetch = AsyncMock(return_value={"id": "PAGE_456", "name": "My Cool Page"}) + + chat = MagicMock() + chat.get_user_name.return_value = "TestBot" + await adapter.initialize(chat) + + assert adapter.bot_user_id == "PAGE_456" + assert adapter.user_name == "My Cool Page" + + @pytest.mark.asyncio + async def test_initialize_preserves_explicit_user_name(self) -> None: + adapter = _make_adapter(user_name="CustomBot") + adapter._graph_api_fetch = AsyncMock(return_value={"id": "PAGE_456", "name": "Page Name"}) + + chat = MagicMock() + chat.get_user_name.return_value = "ChatName" + await adapter.initialize(chat) + + # Explicit user_name wins over both chat.get_user_name() and /me name. + assert adapter.user_name == "CustomBot" + + @pytest.mark.asyncio + async def test_initialize_uses_chat_user_name_when_no_explicit(self) -> None: + adapter = _make_adapter() + adapter._has_explicit_user_name = False + adapter._user_name = "bot" + # /me fails — falls back to chat.get_user_name(). + adapter._graph_api_fetch = AsyncMock(side_effect=RuntimeError("API down")) + + chat = MagicMock() + chat.get_user_name.return_value = "TestBot" + await adapter.initialize(chat) + + assert adapter.bot_user_id is None + assert adapter.user_name == "TestBot" + + @pytest.mark.asyncio + async def test_initialize_continues_when_me_fails(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(side_effect=RuntimeError("API down")) + chat = MagicMock() + chat.get_user_name.return_value = "Bot" + # Should not raise. + await adapter.initialize(chat) + assert adapter.bot_user_id is None + + +# --------------------------------------------------------------------------- +# post_message — text +# --------------------------------------------------------------------------- + + +class TestPostMessageText: + """``post_message`` sends a single text payload via the Send API.""" + + @pytest.mark.asyncio + async def test_post_plain_string(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.s")) + result = await adapter.post_message(THREAD_ID, "Hello!") + + assert result.id == "mid.s" + assert result.thread_id == THREAD_ID + endpoint, kwargs = adapter._graph_api_fetch.call_args.args, adapter._graph_api_fetch.call_args.kwargs + assert endpoint[0] == "me/messages" + assert kwargs["method"] == "POST" + body = kwargs["body"] + assert body["recipient"] == {"id": RECIPIENT_ID} + assert body["message"]["text"] == "Hello!" + assert body["messaging_type"] == "RESPONSE" + + @pytest.mark.asyncio + async def test_post_markdown(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.md")) + await adapter.post_message(THREAD_ID, {"markdown": "**bold** and *italic*"}) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + # Messenger doesn't render markdown — text contains the source. + assert "bold" in body["message"]["text"] + assert "italic" in body["message"]["text"] + + @pytest.mark.asyncio + async def test_post_ast(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.ast")) + await adapter.post_message( + THREAD_ID, + { + "ast": { + "type": "root", + "children": [ + {"type": "paragraph", "children": [{"type": "text", "value": "ast content"}]}, + ], + } + }, + ) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + assert "ast content" in body["message"]["text"] + + @pytest.mark.asyncio + async def test_rejects_empty_message(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response()) + with pytest.raises(ValidationError): + await adapter.post_message(THREAD_ID, " ") + adapter._graph_api_fetch.assert_not_called() + + @pytest.mark.asyncio + async def test_exact_2000_chars_unchanged(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response()) + text = "x" * MESSENGER_MESSAGE_LIMIT + await adapter.post_message(THREAD_ID, text) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + assert body["message"]["text"] == text + assert len(body["message"]["text"]) == MESSENGER_MESSAGE_LIMIT + + @pytest.mark.asyncio + async def test_2001_chars_truncated_with_ellipsis(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response()) + text = "y" * (MESSENGER_MESSAGE_LIMIT + 1) + await adapter.post_message(THREAD_ID, text) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + assert len(body["message"]["text"]) == MESSENGER_MESSAGE_LIMIT + assert body["message"]["text"].endswith("...") + + @pytest.mark.asyncio + async def test_long_text_truncated(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response()) + await adapter.post_message(THREAD_ID, "a" * 3000) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + assert len(body["message"]["text"]) <= MESSENGER_MESSAGE_LIMIT + assert body["message"]["text"].endswith("...") + + @pytest.mark.asyncio + async def test_sent_message_is_cached(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.cached")) + await adapter.post_message(THREAD_ID, "cached msg") + fetched = await adapter.fetch_message(THREAD_ID, "mid.cached") + assert fetched is not None + assert "cached msg" in fetched.text + + @pytest.mark.asyncio + async def test_resolves_bare_psid(self) -> None: + """A non-prefixed value is treated as a raw PSID.""" + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.raw")) + result = await adapter.post_message("USER_123", "hi") + assert result.id == "mid.raw" + + +# --------------------------------------------------------------------------- +# post_message — card templates +# --------------------------------------------------------------------------- + + +class TestPostMessageCard: + """``post_message`` routes cards to generic / button templates or text.""" + + @pytest.mark.asyncio + async def test_generic_template_for_card_with_title(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.tmpl")) + await adapter.post_message( + THREAD_ID, + { + "type": "card", + "title": "Welcome", + "children": [ + {"type": "text", "content": "Hello!"}, + { + "type": "actions", + "children": [ + {"type": "button", "id": "start", "label": "Start"}, + {"type": "button", "id": "help", "label": "Help"}, + ], + }, + ], + }, + ) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + attachment = body["message"]["attachment"] + assert attachment["type"] == "template" + assert attachment["payload"]["template_type"] == "generic" + assert len(attachment["payload"]["elements"]) == 1 + assert attachment["payload"]["elements"][0]["title"] == "Welcome" + assert len(attachment["payload"]["elements"][0]["buttons"]) == 2 + + @pytest.mark.asyncio + async def test_button_template_for_titleless_card(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.btn")) + await adapter.post_message( + THREAD_ID, + { + "type": "card", + "children": [ + {"type": "text", "content": "Please choose:"}, + { + "type": "actions", + "children": [{"type": "button", "id": "opt1", "label": "Option 1"}], + }, + ], + }, + ) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + payload = body["message"]["attachment"]["payload"] + assert payload["template_type"] == "button" + assert payload["text"] == "Please choose:" + + @pytest.mark.asyncio + async def test_text_fallback_for_unsupported_card(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.txt")) + await adapter.post_message( + THREAD_ID, + { + "type": "card", + "title": "With Table", + "children": [ + {"type": "table", "headers": ["A", "B"], "rows": [["1", "2"]]}, + ], + }, + ) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + # Falls back to text — no attachment field. + assert "attachment" not in body["message"] + assert "With Table" in body["message"]["text"] + + +# --------------------------------------------------------------------------- +# stream +# --------------------------------------------------------------------------- + + +class TestStream: + """``stream`` buffers chunks and posts once.""" + + @pytest.mark.asyncio + async def test_buffers_str_chunks(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response("mid.s")) + + async def _chunks() -> AsyncIterator[str | StreamChunk]: + yield "Hello" + yield " " + yield "world" + + result = await adapter.stream(THREAD_ID, _chunks()) + assert result.id == "mid.s" + assert adapter._graph_api_fetch.call_count == 1 + body = adapter._graph_api_fetch.call_args.kwargs["body"] + assert body["message"]["text"] == "Hello world" + + @pytest.mark.asyncio + async def test_buffers_markdown_text_chunks(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=_send_api_response()) + + async def _chunks() -> AsyncIterator[str | StreamChunk]: + yield MarkdownTextChunk(text="Structured ") + yield "plain " + yield MarkdownTextChunk(text="content") + + await adapter.stream(THREAD_ID, _chunks()) + body = adapter._graph_api_fetch.call_args.kwargs["body"] + assert body["message"]["text"] == "Structured plain content" + + +# --------------------------------------------------------------------------- +# Typing indicator +# --------------------------------------------------------------------------- + + +class TestStartTyping: + @pytest.mark.asyncio + async def test_sends_typing_on(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value={}) + await adapter.start_typing(THREAD_ID) + kwargs = adapter._graph_api_fetch.call_args.kwargs + assert kwargs["body"]["sender_action"] == "typing_on" + assert kwargs["body"]["recipient"]["id"] == RECIPIENT_ID + + +# --------------------------------------------------------------------------- +# Unsupported operations +# --------------------------------------------------------------------------- + + +class TestUnsupportedOperations: + """Operations that Messenger doesn't support — must raise ValidationError.""" + + @pytest.mark.asyncio + async def test_edit_message_raises(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + await adapter.edit_message(THREAD_ID, "mid.1", "new text") + + @pytest.mark.asyncio + async def test_delete_message_raises(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + await adapter.delete_message(THREAD_ID, "mid.1") + + @pytest.mark.asyncio + async def test_add_reaction_raises(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + await adapter.add_reaction(THREAD_ID, "mid.1", "thumbsup") + + @pytest.mark.asyncio + async def test_remove_reaction_raises(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + await adapter.remove_reaction(THREAD_ID, "mid.1", "thumbsup") + + +# --------------------------------------------------------------------------- +# Thread / channel info +# --------------------------------------------------------------------------- + + +class TestFetchThreadAndChannel: + """``fetch_thread`` / ``fetch_channel_info`` use the Graph user profile.""" + + @pytest.mark.asyncio + async def test_thread_with_full_name(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value={"id": "USER_123", "first_name": "John", "last_name": "Doe"}) + thread = await adapter.fetch_thread(THREAD_ID) + assert thread.channel_name == "John Doe" + assert thread.is_dm is True + + @pytest.mark.asyncio + async def test_channel_info_with_full_name(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock( + return_value={"id": "USER_123", "first_name": "Jane", "last_name": "Smith"} + ) + info = await adapter.fetch_channel_info(RECIPIENT_ID) + assert info.name == "Jane Smith" + assert info.is_dm is True + + @pytest.mark.asyncio + async def test_falls_back_to_user_id_on_profile_error(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(side_effect=RuntimeError("err")) + info = await adapter.fetch_channel_info(RECIPIENT_ID) + assert info.name == RECIPIENT_ID + + @pytest.mark.asyncio + async def test_first_name_only(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value={"id": "USER_123", "first_name": "Alice"}) + info = await adapter.fetch_thread(THREAD_ID) + assert info.channel_name == "Alice" + + @pytest.mark.asyncio + async def test_last_name_only(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value={"id": "USER_123", "last_name": "Smith"}) + info = await adapter.fetch_thread(THREAD_ID) + assert info.channel_name == "Smith" + + @pytest.mark.asyncio + async def test_caches_user_profile(self) -> None: + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value={"id": "USER_123", "first_name": "John"}) + await adapter.fetch_thread(THREAD_ID) + await adapter.fetch_thread(THREAD_ID) + # Second call hits the cache — no extra API call. + assert adapter._graph_api_fetch.call_count == 1 + + +# --------------------------------------------------------------------------- +# Message fetching / pagination +# --------------------------------------------------------------------------- + + +def _seed_messages(adapter: MessengerAdapter, count: int) -> None: + for i in range(1, count + 1): + adapter.parse_message( + { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000 + i * 1000, + "message": {"mid": f"mid.{i}", "text": f"message {i}"}, + } + ) + + +class TestFetchMessages: + """Local message cache backs ``fetch_messages`` / ``fetch_message``.""" + + @pytest.mark.asyncio + async def test_empty_unknown_thread(self) -> None: + adapter = _make_adapter() + result = await adapter.fetch_messages("messenger:UNKNOWN") + assert result.messages == [] + + @pytest.mark.asyncio + async def test_backward_default(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages(THREAD_ID, FetchOptions(limit=3)) + assert [m.id for m in result.messages] == ["mid.3", "mid.4", "mid.5"] + assert result.next_cursor == "mid.3" + + @pytest.mark.asyncio + async def test_backward_with_cursor(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages( + THREAD_ID, + FetchOptions(limit=2, cursor="mid.3", direction="backward"), + ) + assert [m.id for m in result.messages] == ["mid.1", "mid.2"] + + @pytest.mark.asyncio + async def test_forward(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages( + THREAD_ID, + FetchOptions(limit=2, direction="forward"), + ) + assert [m.id for m in result.messages] == ["mid.1", "mid.2"] + assert result.next_cursor == "mid.2" + + @pytest.mark.asyncio + async def test_forward_with_cursor(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages( + THREAD_ID, + FetchOptions(limit=2, cursor="mid.2", direction="forward"), + ) + assert [m.id for m in result.messages] == ["mid.3", "mid.4"] + + @pytest.mark.asyncio + async def test_no_next_cursor_when_exhausted(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages(THREAD_ID, FetchOptions(limit=100)) + assert len(result.messages) == 5 + assert result.next_cursor is None + + @pytest.mark.asyncio + async def test_clamps_negative_limit(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages(THREAD_ID, FetchOptions(limit=-10)) + assert len(result.messages) == 1 + + @pytest.mark.asyncio + async def test_clamps_high_limit(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages(THREAD_ID, FetchOptions(limit=500)) + assert len(result.messages) == 5 + + @pytest.mark.asyncio + async def test_unknown_cursor_backward(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 3) + result = await adapter.fetch_messages( + THREAD_ID, + FetchOptions(cursor="mid.nonexistent", direction="backward", limit=2), + ) + # Falls back to "from end". + assert [m.id for m in result.messages] == ["mid.2", "mid.3"] + + @pytest.mark.asyncio + async def test_unknown_cursor_forward(self) -> None: + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 3) + result = await adapter.fetch_messages( + THREAD_ID, + FetchOptions(cursor="mid.nonexistent", direction="forward", limit=2), + ) + # Falls back to "from start". + assert [m.id for m in result.messages] == ["mid.1", "mid.2"] + + @pytest.mark.asyncio + async def test_fetch_single_unknown_message_returns_none(self) -> None: + adapter = _make_adapter() + result = await adapter.fetch_message(THREAD_ID, "mid.nope") + assert result is None + + @pytest.mark.asyncio + async def test_sort_by_timestamp_then_sequence(self) -> None: + """Equal timestamps order by the ``:N`` suffix on the mid.""" + adapter = _make_adapter() + adapter.parse_message( + { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.abc:2", "text": "second"}, + } + ) + adapter.parse_message( + { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.abc:1", "text": "first"}, + } + ) + result = await adapter.fetch_messages(THREAD_ID) + assert result.messages[0].text == "first" + assert result.messages[1].text == "second" + + @pytest.mark.asyncio + async def test_reparsing_same_id_updates_cache(self) -> None: + adapter = _make_adapter() + event1 = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.dup", "text": "first"}, + } + event2 = dict(event1) + event2["message"] = {"mid": "mid.dup", "text": "updated"} + adapter.parse_message(event1) + updated = adapter.parse_message(event2) + assert updated.text == "updated" + + +# --------------------------------------------------------------------------- +# parseMessage +# --------------------------------------------------------------------------- + + +class TestParseMessage: + """``parse_message`` produces a normalized ``Message`` from raw events.""" + + def test_basic_message(self) -> None: + adapter = _make_adapter() + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.abc123", "text": "hello"}, + } + parsed = adapter.parse_message(event) + assert parsed.text == "hello" + assert parsed.thread_id == "messenger:USER_123" + assert parsed.id == "mid.abc123" + + def test_is_mention_true(self) -> None: + adapter = _make_adapter() + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.x", "text": "hi"}, + } + parsed = adapter.parse_message(event) + # All inbound Messenger messages are 1:1 DMs — always a mention. + assert parsed.is_mention is True + + def test_echo_marks_as_me_and_bot(self) -> None: + adapter = _make_adapter() + adapter._bot_user_id = "PAGE_456" + event = { + "sender": {"id": "PAGE_456"}, + "recipient": {"id": "USER_123"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.echo", "text": "bot says", "is_echo": True}, + } + parsed = adapter.parse_message(event) + assert parsed.author.is_me is True + assert parsed.author.is_bot is True + + def test_postback_uses_title_as_text(self) -> None: + adapter = _make_adapter() + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "postback": {"title": "Get Started", "payload": "START"}, + } + parsed = adapter.parse_message(event) + assert parsed.id == "event:1735689600000" + assert parsed.text == "Get Started" + + def test_render_formatted_returns_text(self) -> None: + adapter = _make_adapter() + out = adapter.render_formatted( + { + "type": "root", + "children": [ + {"type": "paragraph", "children": [{"type": "text", "value": "hello world"}]}, + ], + } + ) + assert "hello world" in out + + +# --------------------------------------------------------------------------- +# Graph API error mapping +# --------------------------------------------------------------------------- + + +class _FakeResponse: + """Async-context-manager response stub for aiohttp-compatible callers.""" + + def __init__(self, status: int, json_data: Any) -> None: + self.status = status + self._json_data = json_data + + async def json(self, content_type: Any = None) -> Any: + return self._json_data + + async def __aenter__(self) -> _FakeResponse: + return self + + async def __aexit__(self, *_: object) -> None: + return None + + +class _FakeSession: + """Minimal aiohttp-like session stub. Records the last GET/POST call.""" + + def __init__(self, response: _FakeResponse | Exception) -> None: + self._response = response + self.closed = False + self.last_method: str | None = None + + def get(self, url: str, **kwargs: Any) -> Any: + self.last_method = "GET" + if isinstance(self._response, Exception): + raise self._response + return self._response + + def post(self, url: str, **kwargs: Any) -> Any: + self.last_method = "POST" + if isinstance(self._response, Exception): + raise self._response + return self._response + + async def close(self) -> None: + self.closed = True + + +@pytest.mark.asyncio +async def _call_with_response(status: int, json_data: Any) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(status, json_data)) + await adapter.start_typing(THREAD_ID) + + +class TestGraphApiErrors: + """``_graph_api_fetch`` maps Meta error codes to typed adapter errors.""" + + @pytest.mark.asyncio + async def test_rate_limit_on_429(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(429, {"error": {"message": "Rate limited"}})) + with pytest.raises(AdapterRateLimitError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_rate_limit_on_code_4(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(400, {"error": {"message": "Too many calls", "code": 4}})) + with pytest.raises(AdapterRateLimitError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_rate_limit_on_code_32(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(400, {"error": {"message": "Page rate limit", "code": 32}})) + with pytest.raises(AdapterRateLimitError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_rate_limit_on_code_613(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession( + _FakeResponse(400, {"error": {"message": "Custom rate limit", "code": 613}}) + ) + with pytest.raises(AdapterRateLimitError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_auth_error_on_401(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(401, {"error": {"message": "Invalid token", "code": 190}})) + with pytest.raises(AuthenticationError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_auth_error_on_code_190(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(400, {"error": {"message": "Token expired", "code": 190}})) + with pytest.raises(AuthenticationError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_validation_on_403(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession( + _FakeResponse(403, {"error": {"message": "Permission denied", "code": 10}}) + ) + with pytest.raises(ValidationError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_validation_on_code_200(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession( + _FakeResponse(400, {"error": {"message": "Requires permission", "code": 200}}) + ) + with pytest.raises(ValidationError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_not_found_on_404(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(404, {"error": {"message": "Not found"}})) + with pytest.raises(ResourceNotFoundError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_network_on_generic_5xx(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(500, {"error": {"message": "Internal error", "code": 2}})) + with pytest.raises(NetworkError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_network_on_session_exception(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(RuntimeError("DNS failure")) + with pytest.raises(NetworkError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_network_on_unparseable_response(self) -> None: + class _BadResp(_FakeResponse): + async def json(self, content_type: Any = None) -> Any: + raise ValueError("not json") + + adapter = _make_adapter() + adapter._http_session = _FakeSession(_BadResp(200, None)) + with pytest.raises(NetworkError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_fallback_message_when_no_error_message(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(500, {"error": {"code": 999}})) + with pytest.raises(NetworkError, match="Messenger API"): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_uses_status_as_code_when_missing(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(500, {"error": {"message": "Something failed"}})) + with pytest.raises(NetworkError): + await adapter.start_typing(THREAD_ID) + + @pytest.mark.asyncio + async def test_no_error_object(self) -> None: + adapter = _make_adapter() + adapter._http_session = _FakeSession(_FakeResponse(500, {})) + with pytest.raises(NetworkError): + await adapter.start_typing(THREAD_ID) + + +# --------------------------------------------------------------------------- +# Disconnect +# --------------------------------------------------------------------------- + + +class TestDisconnect: + @pytest.mark.asyncio + async def test_disconnect_closes_session(self) -> None: + adapter = _make_adapter() + session = _FakeSession(_FakeResponse(200, {})) + adapter._http_session = session + await adapter.disconnect() + assert session.closed is True + assert adapter._http_session is None + + @pytest.mark.asyncio + async def test_disconnect_when_no_session(self) -> None: + adapter = _make_adapter() + # Should not raise. + await adapter.disconnect() diff --git a/tests/test_messenger_webhook.py b/tests/test_messenger_webhook.py new file mode 100644 index 0000000..5f098ff --- /dev/null +++ b/tests/test_messenger_webhook.py @@ -0,0 +1,851 @@ +"""Tests for the Messenger adapter — webhook routing & signature verification. + +Mirrors the upstream ``packages/adapter-messenger/src/index.test.ts`` suite +focused on the webhook surface: GET verification challenge, POST event +dispatch, X-Hub-Signature-256 verification (valid / invalid / missing / +replay), payload parsing, and event-type routing (messages, echoes, +postbacks, reactions, delivery / read confirmations, mixed batches). + +Pairs with ``tests/test_messenger_api.py`` (Graph API send/stream/error +mapping) — same split used for Telegram and WhatsApp. +""" + +from __future__ import annotations + +import hashlib +import hmac +import json +import os +from dataclasses import dataclass +from typing import Any +from unittest.mock import MagicMock + +import pytest + +from chat_sdk.adapters.messenger.adapter import ( + MessengerAdapter, + create_messenger_adapter, +) +from chat_sdk.adapters.messenger.types import ( + ENV_APP_SECRET, + ENV_PAGE_ACCESS_TOKEN, + ENV_VERIFY_TOKEN, + MessengerAdapterConfig, +) +from chat_sdk.logger import ConsoleLogger +from chat_sdk.shared.errors import ValidationError + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +APP_SECRET = "test-app-secret" +PAGE_TOKEN = "test-page-token" +VERIFY_TOKEN = "test-verify-token" + + +def _make_adapter(**overrides: Any) -> MessengerAdapter: + """Create a ``MessengerAdapter`` with minimal valid config.""" + config_kwargs: dict[str, Any] = { + "app_secret": APP_SECRET, + "page_access_token": PAGE_TOKEN, + "verify_token": VERIFY_TOKEN, + "user_name": "test-bot", + "logger": ConsoleLogger("error"), + } + config_kwargs.update(overrides) + return MessengerAdapter(MessengerAdapterConfig(**config_kwargs)) + + +def _make_chat() -> MagicMock: + """Build a ChatInstance-shaped mock with the methods adapters use.""" + chat = MagicMock() + chat.get_user_name.return_value = "TestBot" + chat.process_message = MagicMock() + chat.process_action = MagicMock() + chat.process_reaction = MagicMock() + return chat + + +@dataclass +class _FakeRequest: + """Minimal request-like object accepted by ``handle_webhook``. + + Mirrors the framework-agnostic duck-typed shape used by the WhatsApp / + Telegram adapters (``url``, ``method``, ``headers``, awaitable ``text``). + """ + + url: str + method: str + _body: str + headers: dict[str, str] + + async def text(self) -> str: # noqa: D102 — async body getter + return self._body + + +def _sign(body: str, secret: str = APP_SECRET) -> str: + """Compute the X-Hub-Signature-256 header value for ``body``.""" + return "sha256=" + hmac.new(secret.encode("utf-8"), body.encode("utf-8"), hashlib.sha256).hexdigest() + + +def _sample_event(**overrides: Any) -> dict[str, Any]: + """Build a representative inbound Messenger messaging event.""" + base: dict[str, Any] = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.abc123", "text": "hello"}, + } + base.update(overrides) + return base + + +def _webhook_payload(events: list[dict[str, Any]]) -> dict[str, Any]: + return { + "object": "page", + "entry": [ + { + "id": "PAGE_456", + "time": 1735689600000, + "messaging": events, + } + ], + } + + +def _post_request(payload: dict[str, Any], *, sign: bool = True, signature: str | None = None) -> _FakeRequest: + body = json.dumps(payload) + headers: dict[str, str] = {"content-type": "application/json"} + if signature is not None: + headers["x-hub-signature-256"] = signature + elif sign: + headers["x-hub-signature-256"] = _sign(body) + return _FakeRequest(url="https://example.com/webhook", method="POST", _body=body, headers=headers) + + +# --------------------------------------------------------------------------- +# Factory and env-fallback behavior (Q1) +# --------------------------------------------------------------------------- + + +@pytest.fixture +def _clear_messenger_env() -> Any: + """Save and clear FACEBOOK_* env vars for the duration of a test.""" + saved: dict[str, str | None] = { + k: os.environ.pop(k, None) for k in (ENV_APP_SECRET, ENV_PAGE_ACCESS_TOKEN, ENV_VERIFY_TOKEN) + } + yield + for k, v in saved.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + + +class TestFactory: + """``create_messenger_adapter`` env fallbacks and Q1 init failure path.""" + + def test_with_explicit_params(self, _clear_messenger_env: Any) -> None: + adapter = create_messenger_adapter( + app_secret="s", + page_access_token="t", + verify_token="v", + user_name="mybot", + ) + assert adapter.name == "messenger" + assert adapter.user_name == "mybot" + + def test_uses_env_vars_when_omitted(self, _clear_messenger_env: Any) -> None: + os.environ[ENV_APP_SECRET] = "secret" + os.environ[ENV_PAGE_ACCESS_TOKEN] = "token" + os.environ[ENV_VERIFY_TOKEN] = "verify" + adapter = create_messenger_adapter() + assert isinstance(adapter, MessengerAdapter) + assert adapter.name == "messenger" + + def test_missing_app_secret_raises(self, _clear_messenger_env: Any) -> None: + os.environ[ENV_PAGE_ACCESS_TOKEN] = "t" + os.environ[ENV_VERIFY_TOKEN] = "v" + with pytest.raises(ValidationError, match="appSecret"): + create_messenger_adapter() + + def test_missing_page_access_token_raises(self, _clear_messenger_env: Any) -> None: + os.environ[ENV_APP_SECRET] = "s" + os.environ[ENV_VERIFY_TOKEN] = "v" + with pytest.raises(ValidationError, match="pageAccessToken"): + create_messenger_adapter() + + def test_missing_verify_token_raises(self, _clear_messenger_env: Any) -> None: + os.environ[ENV_APP_SECRET] = "s" + os.environ[ENV_PAGE_ACCESS_TOKEN] = "t" + with pytest.raises(ValidationError, match="verifyToken"): + create_messenger_adapter() + + def test_constructor_also_raises_on_missing_credentials(self, _clear_messenger_env: Any) -> None: + """Q1: constructing the adapter directly with missing creds also fails. + + The constructor goes through the same ``resolved_*`` fallback chain as + the factory, matching the WhatsApp adapter's behavior and surfacing + config errors loudly at startup rather than at first webhook call. + """ + with pytest.raises(ValidationError, match="appSecret"): + MessengerAdapter( + MessengerAdapterConfig( + page_access_token="t", + verify_token="v", + logger=ConsoleLogger("error"), + ) + ) + + +# --------------------------------------------------------------------------- +# Thread ID encoding +# --------------------------------------------------------------------------- + + +class TestThreadId: + """Encode / decode of Messenger thread IDs.""" + + def test_encode(self) -> None: + from chat_sdk.adapters.messenger.types import MessengerThreadId + + adapter = _make_adapter() + assert adapter.encode_thread_id(MessengerThreadId(recipient_id="USER_123")) == "messenger:USER_123" + + def test_decode(self) -> None: + adapter = _make_adapter() + decoded = adapter.decode_thread_id("messenger:USER_123") + assert decoded.recipient_id == "USER_123" + + def test_decode_rejects_invalid_prefix(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + adapter.decode_thread_id("invalid") + + def test_decode_rejects_empty_recipient(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + adapter.decode_thread_id("messenger:") + + def test_decode_rejects_extra_colons(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + adapter.decode_thread_id("messenger:foo:bar") + + def test_decode_rejects_empty(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + adapter.decode_thread_id("") + + def test_decode_rejects_wrong_platform(self) -> None: + adapter = _make_adapter() + with pytest.raises(ValidationError): + adapter.decode_thread_id("slack:C123:ts") + + def test_is_dm_always_true(self) -> None: + adapter = _make_adapter() + assert adapter.is_dm("messenger:anything") is True + + def test_channel_id_equals_thread_id(self) -> None: + adapter = _make_adapter() + assert adapter.channel_id_from_thread_id("messenger:USER_123") == "messenger:USER_123" + + @pytest.mark.asyncio + async def test_open_dm_returns_encoded(self) -> None: + adapter = _make_adapter() + tid = await adapter.open_dm("USER_999") + assert tid == "messenger:USER_999" + + +# --------------------------------------------------------------------------- +# GET verification challenge (Q3-adjacent — token equality check) +# --------------------------------------------------------------------------- + + +class TestGetVerification: + """Webhook subscription verification (``hub.mode=subscribe``).""" + + @pytest.mark.asyncio + async def test_valid_verification_request(self) -> None: + adapter = _make_adapter() + request = _FakeRequest( + url=f"https://example.com/webhook?hub.mode=subscribe&hub.verify_token={VERIFY_TOKEN}&hub.challenge=CHALLENGE_VALUE", + method="GET", + _body="", + headers={}, + ) + response = await adapter.handle_webhook(request) + assert response["status"] == 200 + assert response["body"] == "CHALLENGE_VALUE" + + @pytest.mark.asyncio + async def test_invalid_verify_token_rejected(self) -> None: + adapter = _make_adapter() + request = _FakeRequest( + url="https://example.com/webhook?hub.mode=subscribe&hub.verify_token=wrong&hub.challenge=CHALLENGE", + method="GET", + _body="", + headers={}, + ) + response = await adapter.handle_webhook(request) + assert response["status"] == 403 + + @pytest.mark.asyncio + async def test_missing_challenge_yields_empty_body(self) -> None: + adapter = _make_adapter() + request = _FakeRequest( + url=f"https://example.com/webhook?hub.mode=subscribe&hub.verify_token={VERIFY_TOKEN}", + method="GET", + _body="", + headers={}, + ) + response = await adapter.handle_webhook(request) + assert response["status"] == 200 + assert response["body"] == "" + + @pytest.mark.asyncio + async def test_wrong_mode_rejected(self) -> None: + adapter = _make_adapter() + request = _FakeRequest( + url=f"https://example.com/webhook?hub.mode=unsubscribe&hub.verify_token={VERIFY_TOKEN}&hub.challenge=CHALLENGE", + method="GET", + _body="", + headers={}, + ) + response = await adapter.handle_webhook(request) + assert response["status"] == 403 + + +# --------------------------------------------------------------------------- +# Signature verification (Q3) +# --------------------------------------------------------------------------- + + +class TestVerifySignature: + """Unit tests for ``_verify_signature``. + + Q3: upstream pins X-Hub-Signature-256 with HMAC-SHA256 over the raw body + using the App Secret. We mirror that exactly; a swappable verifier (like + Slack's ``webhook_verifier``) would diverge from Meta's protocol and + isn't justified for the single-secret Meta integration. + """ + + def test_valid_signature_accepted(self) -> None: + adapter = _make_adapter(app_secret="my-secret") + body = '{"test": true}' + sig = "sha256=" + hmac.new(b"my-secret", body.encode("utf-8"), hashlib.sha256).hexdigest() + assert adapter._verify_signature(body, sig) is True + + def test_invalid_signature_rejected(self) -> None: + adapter = _make_adapter(app_secret="my-secret") + assert adapter._verify_signature('{"a":1}', "sha256=deadbeef") is False + + def test_missing_signature_rejected(self) -> None: + adapter = _make_adapter() + assert adapter._verify_signature("body", None) is False + + def test_empty_signature_rejected(self) -> None: + adapter = _make_adapter() + assert adapter._verify_signature("body", "") is False + + def test_wrong_algo_rejected(self) -> None: + adapter = _make_adapter() + assert adapter._verify_signature("body", "sha1=abcdef") is False + + def test_missing_hash_after_algo_rejected(self) -> None: + adapter = _make_adapter() + assert adapter._verify_signature("body", "sha256=") is False + + def test_signature_without_equals_rejected(self) -> None: + adapter = _make_adapter() + # No "=" separator at all is malformed. + assert adapter._verify_signature("body", "abc123") is False + + def test_signature_with_wrong_secret_rejected(self) -> None: + """A signature computed with the wrong key must not validate. + + Equivalent of a "replay with attacker's secret" / forgery attempt — + even with a valid-looking sha256 hex shape, mismatching HMAC fails. + """ + adapter = _make_adapter(app_secret="real-secret") + body = '{"x":1}' + forged = "sha256=" + hmac.new(b"other-secret", body.encode("utf-8"), hashlib.sha256).hexdigest() + assert adapter._verify_signature(body, forged) is False + + def test_replay_with_modified_body_rejected(self) -> None: + """Capturing a signature and replaying it with a mutated body fails. + + Models the classic "attacker replays old signature against new body" + scenario — HMAC binds the signature to the exact bytes that were + signed, so any change to the body invalidates the signature. + """ + adapter = _make_adapter() + original_body = json.dumps(_webhook_payload([_sample_event()])) + sig = _sign(original_body) + # Same signature, body mutated (e.g. attacker injects another event). + mutated_body = json.dumps(_webhook_payload([_sample_event(), _sample_event()])) + assert adapter._verify_signature(mutated_body, sig) is False + + +# --------------------------------------------------------------------------- +# POST webhook — payload validation & signature gate +# --------------------------------------------------------------------------- + + +class TestWebhookPostSignatureGate: + """End-to-end signature gating in ``handle_webhook`` (POST).""" + + @pytest.mark.asyncio + async def test_missing_signature_header_rejected(self) -> None: + adapter = _make_adapter() + request = _post_request(_webhook_payload([_sample_event()]), sign=False) + response = await adapter.handle_webhook(request) + assert response["status"] == 403 + + @pytest.mark.asyncio + async def test_wrong_algo_rejected(self) -> None: + adapter = _make_adapter() + request = _post_request(_webhook_payload([_sample_event()]), signature="sha1=abc123") + response = await adapter.handle_webhook(request) + assert response["status"] == 403 + + @pytest.mark.asyncio + async def test_missing_hash_rejected(self) -> None: + adapter = _make_adapter() + request = _post_request(_webhook_payload([_sample_event()]), signature="sha256=") + response = await adapter.handle_webhook(request) + assert response["status"] == 403 + + @pytest.mark.asyncio + async def test_bad_hex_rejected(self) -> None: + adapter = _make_adapter() + request = _post_request(_webhook_payload([_sample_event()]), signature="sha256=not-valid-hex") + response = await adapter.handle_webhook(request) + assert response["status"] == 403 + + @pytest.mark.asyncio + async def test_valid_signature_returns_event_received(self) -> None: + adapter = _make_adapter() + adapter._chat = _make_chat() # bypass full init + request = _post_request(_webhook_payload([_sample_event()])) + response = await adapter.handle_webhook(request) + assert response["status"] == 200 + assert response["body"] == "EVENT_RECEIVED" + + +class TestWebhookPostPayloadValidation: + """Payload-shape validation after signature passes.""" + + @pytest.mark.asyncio + async def test_invalid_json_returns_400(self) -> None: + adapter = _make_adapter() + body = "not valid json{{{" + request = _FakeRequest( + url="https://example.com/webhook", + method="POST", + _body=body, + headers={"content-type": "application/json", "x-hub-signature-256": _sign(body)}, + ) + response = await adapter.handle_webhook(request) + assert response["status"] == 400 + + @pytest.mark.asyncio + async def test_non_page_object_returns_404(self) -> None: + adapter = _make_adapter() + request = _post_request({"object": "user", "entry": []}) + response = await adapter.handle_webhook(request) + assert response["status"] == 404 + + @pytest.mark.asyncio + async def test_uninitialized_chat_returns_200_and_warns(self) -> None: + """Match upstream: webhooks that arrive before ``initialize`` ack 200. + + Returning non-200 would cause Meta to retry — and the only thing the + adapter knows at this point is that it's not ready to dispatch, not + that the payload is bad. Acking matches upstream. + """ + adapter = _make_adapter() + request = _post_request(_webhook_payload([_sample_event()])) + response = await adapter.handle_webhook(request) + assert response["status"] == 200 + assert response["body"] == "EVENT_RECEIVED" + + +# --------------------------------------------------------------------------- +# Event routing +# --------------------------------------------------------------------------- + + +class TestWebhookMessageRouting: + """Routing of inbound message events to ``chat.process_message``.""" + + @pytest.mark.asyncio + async def test_incoming_message_dispatched(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + request = _post_request(_webhook_payload([_sample_event()])) + response = await adapter.handle_webhook(request) + assert response["status"] == 200 + assert chat.process_message.call_count == 1 + call_args = chat.process_message.call_args + # Signature: (adapter, thread_id, message, options) + assert call_args.args[1] == "messenger:USER_123" + + @pytest.mark.asyncio + async def test_echo_message_not_dispatched(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = _sample_event(message={"mid": "mid.echo", "text": "bot", "is_echo": True}) + request = _post_request(_webhook_payload([event])) + await adapter.handle_webhook(request) + assert chat.process_message.call_count == 0 + + @pytest.mark.asyncio + async def test_echo_message_cached_for_history(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = _sample_event( + sender={"id": "PAGE_456"}, + recipient={"id": "USER_123"}, + message={"mid": "mid.echo1", "text": "bot reply", "is_echo": True}, + ) + request = _post_request(_webhook_payload([event])) + await adapter.handle_webhook(request) + cached = await adapter.fetch_message("messenger:USER_123", "mid.echo1") + assert cached is not None + assert cached.text == "bot reply" + + @pytest.mark.asyncio + async def test_multiple_messages_in_entry(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + payload = _webhook_payload( + [ + _sample_event(message={"mid": "mid.1", "text": "first"}), + _sample_event(message={"mid": "mid.2", "text": "second"}), + _sample_event(message={"mid": "mid.3", "text": "third"}), + ] + ) + await adapter.handle_webhook(_post_request(payload)) + assert chat.process_message.call_count == 3 + + @pytest.mark.asyncio + async def test_multiple_entries_in_payload(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + payload = { + "object": "page", + "entry": [ + { + "id": "PAGE_456", + "time": 1735689600000, + "messaging": [_sample_event(message={"mid": "mid.a", "text": "from 1"})], + }, + { + "id": "PAGE_456", + "time": 1735689601000, + "messaging": [_sample_event(message={"mid": "mid.b", "text": "from 2"})], + }, + ], + } + await adapter.handle_webhook(_post_request(payload)) + assert chat.process_message.call_count == 2 + + @pytest.mark.asyncio + async def test_delivery_confirmation_no_error(self) -> None: + adapter = _make_adapter() + adapter._chat = _make_chat() + event = _sample_event(message=None, delivery={"watermark": 1735689600000, "mids": ["mid.abc"]}) + # The TypedDict allows None for missing keys; remove ``message`` key. + event.pop("message") + response = await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + assert response["status"] == 200 + + @pytest.mark.asyncio + async def test_read_confirmation_no_error(self) -> None: + adapter = _make_adapter() + adapter._chat = _make_chat() + event = _sample_event(read={"watermark": 1735689600000}) + event.pop("message") + response = await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + assert response["status"] == 200 + + @pytest.mark.asyncio + async def test_mixed_event_types(self) -> None: + """Single payload with message + reaction + delivery + read.""" + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + payload = _webhook_payload( + [ + _sample_event(message={"mid": "mid.msg", "text": "hi"}), + { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "reaction": { + "mid": "mid.msg", + "action": "react", + "emoji": "❤", + "reaction": "love", + }, + }, + { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "delivery": {"watermark": 1735689600000, "mids": ["mid.msg"]}, + }, + { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "read": {"watermark": 1735689600000}, + }, + ] + ) + response = await adapter.handle_webhook(_post_request(payload)) + assert response["status"] == 200 + assert chat.process_message.call_count == 1 + assert chat.process_reaction.call_count == 1 + + +class TestPostbackRouting: + """Postback events dispatch via ``chat.process_action``.""" + + @pytest.mark.asyncio + async def test_postback_dispatched(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "postback": {"title": "Get Started", "payload": "GET_STARTED"}, + } + await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + assert chat.process_action.call_count == 1 + + @pytest.mark.asyncio + async def test_postback_mid_used_when_present(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "postback": {"title": "Menu", "payload": "MENU_1", "mid": "mid.postback1"}, + } + await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + action = chat.process_action.call_args.args[0] + assert action.message_id == "mid.postback1" + assert action.action_id == "MENU_1" + # Q2 (PR 1): callback-data passthrough — when no chat: prefix, both + # action_id and value come from the raw payload. + assert action.value == "MENU_1" + + @pytest.mark.asyncio + async def test_postback_falls_back_to_timestamp_id(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689999000, + "postback": {"title": "Get Started", "payload": "GET_STARTED"}, + } + await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + action = chat.process_action.call_args.args[0] + assert action.message_id == "postback:1735689999000" + + @pytest.mark.asyncio + async def test_postback_with_chat_prefixed_payload(self) -> None: + """Postbacks with the ``chat:`` prefix decode into (action, value).""" + from chat_sdk.adapters.messenger.cards import encode_messenger_callback_data + + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + encoded = encode_messenger_callback_data("approve", "yes") + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "postback": {"title": "Approve", "payload": encoded, "mid": "mid.p"}, + } + await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + action = chat.process_action.call_args.args[0] + assert action.action_id == "approve" + assert action.value == "yes" + + +class TestReactionRouting: + """Reaction events dispatch via ``chat.process_reaction``.""" + + @pytest.mark.asyncio + async def test_react_event(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "reaction": { + "mid": "m_reacted", + "action": "react", + "emoji": "❤", + "reaction": "love", + }, + } + await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + reaction = chat.process_reaction.call_args.args[0] + assert reaction.message_id == "m_reacted" + assert reaction.raw_emoji == "❤" + assert reaction.added is True + + @pytest.mark.asyncio + async def test_unreact_event(self) -> None: + adapter = _make_adapter() + chat = _make_chat() + adapter._chat = chat + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "reaction": { + "mid": "m_reacted", + "action": "unreact", + "emoji": "❤", + "reaction": "love", + }, + } + await adapter.handle_webhook(_post_request(_webhook_payload([event]))) + reaction = chat.process_reaction.call_args.args[0] + assert reaction.added is False + + +# --------------------------------------------------------------------------- +# Attachment parsing (no network) +# --------------------------------------------------------------------------- + + +class TestAttachmentParsing: + """``parse_message`` extracts attachments from inbound events.""" + + def test_extracts_image_video_audio_file_fallback(self) -> None: + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.attach", + "text": "check", + "attachments": [ + {"type": "image", "payload": {"url": "https://example.com/img.jpg"}}, + {"type": "video", "payload": {"url": "https://example.com/vid.mp4"}}, + {"type": "audio", "payload": {"url": "https://example.com/aud.mp3"}}, + {"type": "file", "payload": {"url": "https://example.com/doc.pdf"}}, + {"type": "fallback", "payload": {"url": "https://example.com/fb"}}, + ], + } + ) + parsed = adapter.parse_message(event) + assert len(parsed.attachments) == 5 + assert [a.type for a in parsed.attachments] == ["image", "video", "audio", "file", "file"] + + def test_skips_attachments_without_url(self) -> None: + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.nourl", + "text": "sticker", + "attachments": [ + {"type": "image", "payload": {"sticker_id": 123}}, + {"type": "image"}, + ], + } + ) + parsed = adapter.parse_message(event) + assert parsed.attachments == [] + + def test_location_attachment_mapped_to_file(self) -> None: + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.loc", + "text": "location", + "attachments": [ + {"type": "location", "payload": {"url": "https://maps.example.com/x"}}, + ], + } + ) + parsed = adapter.parse_message(event) + assert len(parsed.attachments) == 1 + assert parsed.attachments[0].type == "file" + + def test_no_attachments_field(self) -> None: + adapter = _make_adapter() + event = _sample_event(message={"mid": "mid.no", "text": "plain"}) + parsed = adapter.parse_message(event) + assert parsed.attachments == [] + + def test_attachment_has_fetch_data_callable(self) -> None: + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.fd", + "text": "x", + "attachments": [{"type": "image", "payload": {"url": "https://example.com/img.jpg"}}], + } + ) + parsed = adapter.parse_message(event) + assert parsed.attachments[0].fetch_data is not None + assert callable(parsed.attachments[0].fetch_data) + + @pytest.mark.asyncio + async def test_attachment_download_uses_session(self) -> None: + """``fetch_data`` reads bytes from the shared aiohttp session.""" + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.dl", + "text": "x", + "attachments": [{"type": "image", "payload": {"url": "https://example.com/img.jpg"}}], + } + ) + parsed = adapter.parse_message(event) + + # Patch the shared session with a minimal context-manager mock. + class _Resp: + status = 200 + + async def read(self) -> bytes: + return b"image-bytes" + + async def __aenter__(self) -> _Resp: + return self + + async def __aexit__(self, *_: object) -> None: + pass + + # aiohttp's session.get(url) is a sync call returning an + # async-context-manager request handle. Wire side_effect (instead of + # assigning a fresh mock to .get) so the call returns _Resp directly. + def _session_get(url: str, **_kw: object) -> _Resp: + return _Resp() + + mock_session = MagicMock() + mock_session.closed = False + mock_session.get.side_effect = _session_get + adapter._http_session = mock_session + + result = await parsed.attachments[0].fetch_data() + assert result == b"image-bytes" From 36715cada54a79ca31614e157a1a82a54a5f7704 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 21:05:06 +0000 Subject: [PATCH 2/7] fix(messenger): verify HMAC on raw bytes + URL/profile defenses (gemini review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four follow-ups to PR #124 from Gemini's review of the Messenger adapter: - `_handle_verification` casts `request.url` to `str` before `urlparse`. Starlette/yarl `URL` objects are non-`str` and would raise. - `_get_request_body` now returns `bytes` instead of `str`. Decoding to UTF-8 and re-encoding for the HMAC step risks replacement characters (U+FFFD) for any non-UTF-8 byte sequence — that breaks signature parity with Meta's reference implementation and silently rejects legitimate webhooks. Body sources are also reordered: `body` attribute first (canonical raw bytes on Starlette/Django), `text` attribute fallback (aiohttp's str path). `is not None` everywhere so a legitimately empty `b""` body isn't skipped. - `_verify_signature(body: bytes, ...)` matches the new contract; the `body.encode("utf-8")` step is dropped — HMAC operates on the exact wire bytes Meta signed. JSON parsing at the call site still works (`json.loads` accepts both str and bytes). - `_fetch_user_profile` now `isinstance(profile, dict)` checks the Graph API response before caching. A non-dict response (None, list, unexpected shape) used to poison `_user_profile_cache` and raise `AttributeError` on the next `.get` call in `_profile_display_name`; we now fall back to the minimal `{"id": user_id}` profile and leave the cache untouched. Regression coverage (+7 tests): - `TestVerifySignature::test_verifies_raw_bytes_without_encoding_roundtrip` feeds a body with lone continuation bytes (0x80 0xff) that don't survive UTF-8 round-trip. Fails on the old decode+re-encode path. - `TestGetRequestBody` (new class, 5 cases) pins the bytes return type across all four framework-shaped inputs: bytes body attribute, str body attribute, async-callable body (Starlette/FastAPI), async-callable text (aiohttp), and missing-body → `b""`. - `test_non_dict_profile_response_falls_back` exercises both `None` and list responses, asserting the fallback display name and that the cache stays empty. Existing `TestVerifySignature` cases updated to pass `bytes` bodies, matching the new contract. `_sign` test helper accepts `bytes | str` for convenience. https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj --- src/chat_sdk/adapters/messenger/adapter.py | 39 ++++--- tests/test_messenger_api.py | 24 ++++ tests/test_messenger_webhook.py | 129 ++++++++++++++++++--- 3 files changed, 164 insertions(+), 28 deletions(-) diff --git a/src/chat_sdk/adapters/messenger/adapter.py b/src/chat_sdk/adapters/messenger/adapter.py index de151df..148f2f3 100644 --- a/src/chat_sdk/adapters/messenger/adapter.py +++ b/src/chat_sdk/adapters/messenger/adapter.py @@ -287,7 +287,7 @@ async def handle_webhook( def _handle_verification(self, request: Any) -> Any: """Handle the GET subscription challenge.""" url = getattr(request, "url", "") - parsed = urlparse(url) + parsed = urlparse(str(url)) params = parse_qs(parsed.query) mode = (params.get("hub.mode") or [None])[0] @@ -301,7 +301,7 @@ def _handle_verification(self, request: Any) -> Any: self._logger.warn("Messenger webhook verification failed") return self._make_response("Forbidden", 403) - def _verify_signature(self, body: str, signature: str | None) -> bool: + def _verify_signature(self, body: bytes, signature: str | None) -> bool: """Verify the ``X-Hub-Signature-256`` header (HMAC-SHA256, App Secret). Format: ``sha256=``. Returns ``False`` on any malformed input. @@ -325,7 +325,7 @@ def _verify_signature(self, body: str, signature: str | None) -> bool: try: computed_hex = hmac.new( self._app_secret.encode("utf-8"), - body.encode("utf-8"), + body, hashlib.sha256, ).hexdigest() # Compare hex strings directly. The signature is presented as a @@ -854,6 +854,8 @@ async def _fetch_user_profile(self, user_id: str) -> MessengerUserProfile: method="GET", query_params={"fields": "first_name,last_name,profile_pic"}, ) + if not isinstance(profile, dict): + return {"id": user_id} self._user_profile_cache[user_id] = profile return cast(MessengerUserProfile, profile) except Exception: @@ -1033,23 +1035,32 @@ def _throw_graph_api_error( # ========================================================================= @staticmethod - async def _get_request_body(request: Any) -> str: - """Extract body text from a request object (sync or async).""" - text_attr = getattr(request, "text", None) - if text_attr is not None: - if callable(text_attr): - result = text_attr() - text_attr = await result if inspect.isawaitable(result) else result - return text_attr.decode("utf-8") if isinstance(text_attr, (bytes, bytearray)) else str(text_attr) + async def _get_request_body(request: Any) -> bytes: + """Extract the raw request body as bytes (sync or async). + + Returned as raw bytes so signature verification operates on the exact + wire bytes Meta signed. Any encode/decode round-trip risks replacement + characters or altered code points and breaks HMAC parity. + """ body = getattr(request, "body", None) if body is not None: if callable(body): result = body() body = await result if inspect.isawaitable(result) else result if isinstance(body, (bytes, bytearray)): - return body.decode("utf-8") - return str(body) - return "" + return bytes(body) + if isinstance(body, str): + return body.encode("utf-8") + text_attr = getattr(request, "text", None) + if text_attr is not None: + if callable(text_attr): + result = text_attr() + text_attr = await result if inspect.isawaitable(result) else result + if isinstance(text_attr, (bytes, bytearray)): + return bytes(text_attr) + if isinstance(text_attr, str): + return text_attr.encode("utf-8") + return b"" @staticmethod def _get_header(request: Any, name: str) -> str | None: diff --git a/tests/test_messenger_api.py b/tests/test_messenger_api.py index f29f63b..4fdac44 100644 --- a/tests/test_messenger_api.py +++ b/tests/test_messenger_api.py @@ -451,6 +451,30 @@ async def test_caches_user_profile(self) -> None: # Second call hits the cache — no extra API call. assert adapter._graph_api_fetch.call_count == 1 + @pytest.mark.asyncio + async def test_non_dict_profile_response_falls_back(self) -> None: + """A successful Graph API call that returns a non-mapping must not poison the cache. + + ``_graph_api_fetch`` is typed loosely and could in principle yield + ``None`` or a list (e.g. an API shape change, a stubbed test, a + proxy returning an array). Without the ``isinstance(profile, dict)`` + guard, ``_profile_display_name`` then calls ``.get`` on whatever + came back and raises ``AttributeError``, and the next call hits the + same poisoned cache entry. Verify (a) the minimal ``{"id": uid}`` + fallback is returned, and (b) the cache stays empty. + """ + adapter = _make_adapter() + adapter._graph_api_fetch = AsyncMock(return_value=None) + info = await adapter.fetch_thread(THREAD_ID) + assert info.channel_name == RECIPIENT_ID + assert RECIPIENT_ID not in adapter._user_profile_cache + + # Same again for a list, to pin the dict-only contract. + adapter._graph_api_fetch = AsyncMock(return_value=[{"id": RECIPIENT_ID}]) + info = await adapter.fetch_thread(THREAD_ID) + assert info.channel_name == RECIPIENT_ID + assert RECIPIENT_ID not in adapter._user_profile_cache + # --------------------------------------------------------------------------- # Message fetching / pagination diff --git a/tests/test_messenger_webhook.py b/tests/test_messenger_webhook.py index 5f098ff..9db22e8 100644 --- a/tests/test_messenger_webhook.py +++ b/tests/test_messenger_webhook.py @@ -84,9 +84,10 @@ async def text(self) -> str: # noqa: D102 — async body getter return self._body -def _sign(body: str, secret: str = APP_SECRET) -> str: +def _sign(body: bytes | str, secret: str = APP_SECRET) -> str: """Compute the X-Hub-Signature-256 header value for ``body``.""" - return "sha256=" + hmac.new(secret.encode("utf-8"), body.encode("utf-8"), hashlib.sha256).hexdigest() + body_bytes = body if isinstance(body, bytes) else body.encode("utf-8") + return "sha256=" + hmac.new(secret.encode("utf-8"), body_bytes, hashlib.sha256).hexdigest() def _sample_event(**overrides: Any) -> dict[str, Any]: @@ -333,34 +334,34 @@ class TestVerifySignature: def test_valid_signature_accepted(self) -> None: adapter = _make_adapter(app_secret="my-secret") - body = '{"test": true}' - sig = "sha256=" + hmac.new(b"my-secret", body.encode("utf-8"), hashlib.sha256).hexdigest() + body = b'{"test": true}' + sig = "sha256=" + hmac.new(b"my-secret", body, hashlib.sha256).hexdigest() assert adapter._verify_signature(body, sig) is True def test_invalid_signature_rejected(self) -> None: adapter = _make_adapter(app_secret="my-secret") - assert adapter._verify_signature('{"a":1}', "sha256=deadbeef") is False + assert adapter._verify_signature(b'{"a":1}', "sha256=deadbeef") is False def test_missing_signature_rejected(self) -> None: adapter = _make_adapter() - assert adapter._verify_signature("body", None) is False + assert adapter._verify_signature(b"body", None) is False def test_empty_signature_rejected(self) -> None: adapter = _make_adapter() - assert adapter._verify_signature("body", "") is False + assert adapter._verify_signature(b"body", "") is False def test_wrong_algo_rejected(self) -> None: adapter = _make_adapter() - assert adapter._verify_signature("body", "sha1=abcdef") is False + assert adapter._verify_signature(b"body", "sha1=abcdef") is False def test_missing_hash_after_algo_rejected(self) -> None: adapter = _make_adapter() - assert adapter._verify_signature("body", "sha256=") is False + assert adapter._verify_signature(b"body", "sha256=") is False def test_signature_without_equals_rejected(self) -> None: adapter = _make_adapter() # No "=" separator at all is malformed. - assert adapter._verify_signature("body", "abc123") is False + assert adapter._verify_signature(b"body", "abc123") is False def test_signature_with_wrong_secret_rejected(self) -> None: """A signature computed with the wrong key must not validate. @@ -369,8 +370,8 @@ def test_signature_with_wrong_secret_rejected(self) -> None: even with a valid-looking sha256 hex shape, mismatching HMAC fails. """ adapter = _make_adapter(app_secret="real-secret") - body = '{"x":1}' - forged = "sha256=" + hmac.new(b"other-secret", body.encode("utf-8"), hashlib.sha256).hexdigest() + body = b'{"x":1}' + forged = "sha256=" + hmac.new(b"other-secret", body, hashlib.sha256).hexdigest() assert adapter._verify_signature(body, forged) is False def test_replay_with_modified_body_rejected(self) -> None: @@ -381,12 +382,112 @@ def test_replay_with_modified_body_rejected(self) -> None: signed, so any change to the body invalidates the signature. """ adapter = _make_adapter() - original_body = json.dumps(_webhook_payload([_sample_event()])) + original_body = json.dumps(_webhook_payload([_sample_event()])).encode("utf-8") sig = _sign(original_body) # Same signature, body mutated (e.g. attacker injects another event). - mutated_body = json.dumps(_webhook_payload([_sample_event(), _sample_event()])) + mutated_body = json.dumps(_webhook_payload([_sample_event(), _sample_event()])).encode("utf-8") assert adapter._verify_signature(mutated_body, sig) is False + def test_verifies_raw_bytes_without_encoding_roundtrip(self) -> None: + """Signature must be checked against the exact wire bytes. + + Meta signs the raw HTTP request body. If the adapter re-encodes the + body (decode-to-str then encode-back-to-bytes) before HMAC, any byte + sequence that's not valid UTF-8 gets replaced with U+FFFD and the + computed HMAC diverges from Meta's — legitimate webhooks would fail + verification. This regression pins the contract: feed a body whose + bytes don't round-trip through UTF-8 cleanly, and verification still + succeeds when the signature was computed over the original bytes. + """ + adapter = _make_adapter(app_secret="my-secret") + # Lone continuation byte (0x80) — invalid UTF-8. A decode+re-encode + # round-trip via "utf-8" with errors="replace" would mutate this to + # the 3-byte U+FFFD sequence, breaking HMAC parity. + body = b'{"data":"\x80\xff"}' + assert body.decode("utf-8", errors="replace").encode("utf-8") != body + sig = "sha256=" + hmac.new(b"my-secret", body, hashlib.sha256).hexdigest() + assert adapter._verify_signature(body, sig) is True + + +class TestGetRequestBody: + """Pin that ``_get_request_body`` returns raw bytes. + + The function feeds ``_verify_signature``; switching its return type + would silently re-introduce the decode/re-encode hazard. These tests + cover the four frameworks-shaped inputs the helper supports + (bytes/str ``body`` attribute, bytes/str ``text`` attribute, async or + sync callables) and pin the bytes contract on each path. + """ + + @pytest.mark.asyncio + async def test_bytes_body_attribute_returned_unchanged(self) -> None: + adapter = _make_adapter() + raw = b'{"x":"\x80\xff"}' + + class Req: + body = raw + + result = await adapter._get_request_body(Req()) + assert result == raw + assert isinstance(result, bytes) + + @pytest.mark.asyncio + async def test_str_body_attribute_encoded_utf8(self) -> None: + adapter = _make_adapter() + + class Req: + body = '{"hello": "world"}' + + result = await adapter._get_request_body(Req()) + assert result == b'{"hello": "world"}' + assert isinstance(result, bytes) + + @pytest.mark.asyncio + async def test_async_callable_body_returns_bytes(self) -> None: + """Mirrors Starlette/FastAPI: ``await request.body()`` returns bytes.""" + adapter = _make_adapter() + raw = b'{"async": true}' + + async def body() -> bytes: + return raw + + class Req: + pass + + req = Req() + req.body = body # type: ignore[attr-defined] + result = await adapter._get_request_body(req) + assert result == raw + assert isinstance(result, bytes) + + @pytest.mark.asyncio + async def test_text_attribute_fallback_returns_bytes(self) -> None: + """aiohttp-style: ``await request.text()`` returns str — encode it.""" + adapter = _make_adapter() + + async def text() -> str: + return '{"text": "ok"}' + + class Req: + pass + + req = Req() + req.text = text # type: ignore[attr-defined] + result = await adapter._get_request_body(req) + assert result == b'{"text": "ok"}' + assert isinstance(result, bytes) + + @pytest.mark.asyncio + async def test_missing_body_returns_empty_bytes(self) -> None: + adapter = _make_adapter() + + class Req: + pass + + result = await adapter._get_request_body(Req()) + assert result == b"" + assert isinstance(result, bytes) + # --------------------------------------------------------------------------- # POST webhook — payload validation & signature gate From c976de38ba1b0f95185f1819f32f309de667e1c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 00:02:56 +0000 Subject: [PATCH 3/7] fix(messenger): accept uppercase-hex signatures + clarify init-failure docstring (review) https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj --- src/chat_sdk/adapters/messenger/adapter.py | 20 ++++++++++++-------- tests/test_messenger_webhook.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/chat_sdk/adapters/messenger/adapter.py b/src/chat_sdk/adapters/messenger/adapter.py index 148f2f3..77500b8 100644 --- a/src/chat_sdk/adapters/messenger/adapter.py +++ b/src/chat_sdk/adapters/messenger/adapter.py @@ -328,9 +328,11 @@ def _verify_signature(self, body: bytes, signature: str | None) -> bool: body, hashlib.sha256, ).hexdigest() - # Compare hex strings directly. The signature is presented as a - # lowercase hex string by Meta; ``hexdigest()`` is lowercase. - return hmac.compare_digest(hash_hex, computed_hex) + # Compare hex strings. ``hexdigest()`` is lowercase; Node's + # ``Buffer.from(hex)`` is case-insensitive, so upstream accepts an + # uppercase-hex signature. Normalize the header hash to lowercase + # before the constant-time compare to match that behavior. + return hmac.compare_digest(hash_hex.lower(), computed_hex) except Exception: self._logger.warn("Failed to verify Messenger webhook signature") return False @@ -1097,11 +1099,13 @@ def create_messenger_adapter( ) -> MessengerAdapter: """Factory for ``MessengerAdapter`` with env-var fallbacks. - Q1 (see #110): init-failure behavior. We match the upstream contract and - the sibling ``WhatsAppAdapter`` by raising ``ValidationError`` at - construction time for each missing required credential. This keeps the - Meta-family adapters consistent and surfaces config errors loudly during - startup rather than at first webhook call. + Q1 (see #110): init-failure behavior. We match the upstream contract by + raising ``ValidationError`` at construction time for each missing required + credential. This improves on the sibling ``WhatsAppAdapter``, whose + constructor does no validation (direct misconstruction raises ``TypeError``, + not ``ValidationError``) -- Messenger raises a descriptive ``ValidationError`` + from both the factory and the constructor. It surfaces config errors loudly + during startup rather than at first webhook call. """ _logger = logger or ConsoleLogger("info").child("messenger") diff --git a/tests/test_messenger_webhook.py b/tests/test_messenger_webhook.py index 9db22e8..a7d8e5e 100644 --- a/tests/test_messenger_webhook.py +++ b/tests/test_messenger_webhook.py @@ -338,6 +338,20 @@ def test_valid_signature_accepted(self) -> None: sig = "sha256=" + hmac.new(b"my-secret", body, hashlib.sha256).hexdigest() assert adapter._verify_signature(body, sig) is True + def test_uppercase_hex_signature_accepted(self) -> None: + """An uppercase-hex signature must still verify. + + ``hexdigest()`` is lowercase, but Node's ``Buffer.from(hex)`` is + case-insensitive, so upstream accepts an uppercase-hex signature. + We normalize the header hash to lowercase before the constant-time + compare; without that, this exact-but-uppercased signature would be + rejected. Pins parity with upstream's behavior. + """ + adapter = _make_adapter(app_secret="my-secret") + body = b'{"test": true}' + sig = "sha256=" + hmac.new(b"my-secret", body, hashlib.sha256).hexdigest().upper() + assert adapter._verify_signature(body, sig) is True + def test_invalid_signature_rejected(self) -> None: adapter = _make_adapter(app_secret="my-secret") assert adapter._verify_signature(b'{"a":1}', "sha256=deadbeef") is False From a0a532c56ba8e99e5443c2424b017ff44c6cb034 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 00:22:50 +0000 Subject: [PATCH 4/7] fix(messenger): implement rehydrate_attachment for queue/debounce safety (codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2: Messenger attachments processed under queue/debounce/burst concurrency lose their fetch_data closure during JSON serialization in the state backend. Without a rehydrate_attachment hook, dequeued handlers see attachment.fetch_data is None and cannot download the file. Mirrors WhatsAppAdapter.rehydrate_attachment (same Meta family, same queue-mode failure shape): - _extract_attachments now persists the download URL on attachment.fetch_metadata={"url": url} - New rehydrate_attachment reads that URL and rebuilds the lazy downloader via _make_attachment_downloader, reusing the shared aiohttp session. Returns the attachment unchanged when metadata is missing/incomplete (degraded mode, matches the documented "leave unchanged when no hook" behavior). No auth headers are attached by the rebuilt closure — Messenger payload URLs are signature-gated by Meta and the original _download_attachment already operated without Bearer tokens. Upstream parity: vercel/chat's TS adapter-messenger does not implement this hook because queue mode is Python-only. Tests (4 new, load-bearing): - fetch_metadata carries the URL after extraction - queue/serialize roundtrip + rehydrate restores a working downloader that hits the original URL (would fail without the hook — verified by temporary revert) - two degraded-mode pins (no metadata, metadata without url key) --- src/chat_sdk/adapters/messenger/adapter.py | 42 ++++++++ tests/test_messenger_webhook.py | 114 +++++++++++++++++++++ 2 files changed, 156 insertions(+) diff --git a/src/chat_sdk/adapters/messenger/adapter.py b/src/chat_sdk/adapters/messenger/adapter.py index 77500b8..8ae7bc2 100644 --- a/src/chat_sdk/adapters/messenger/adapter.py +++ b/src/chat_sdk/adapters/messenger/adapter.py @@ -793,6 +793,13 @@ def _extract_attachments(self, event: MessengerMessagingEvent) -> list[Attachmen type=self._map_attachment_type(attachment.get("type", "")), url=url, fetch_data=self._make_attachment_downloader(url), + # Persist the download URL so the closure can be rebuilt + # by ``rehydrate_attachment`` after the message survives + # a queue/debounce/burst JSON roundtrip (which drops the + # ``fetch_data`` closure). Messenger payload URLs are + # signature-gated by Meta and require no auth header, so + # the URL alone is sufficient to rebuild the closure. + fetch_metadata={"url": url}, ) ) return result @@ -810,6 +817,41 @@ async def _download() -> bytes: return _download + def rehydrate_attachment(self, attachment: Attachment) -> Attachment: + """Reconstruct ``fetch_data`` on a deserialized Messenger attachment. + + Called by :class:`~chat_sdk.chat.Chat` during message rehydration in + the queue/debounce/burst concurrency paths, where the original + ``fetch_data`` closure was dropped during JSON serialization. Reads + the download URL from ``attachment.fetch_metadata`` (populated by + :meth:`_extract_attachments`) and rebuilds the lazy downloader that + reuses the shared aiohttp session. Returns the attachment unchanged + when no URL is present — matching the upstream documented "leave + unchanged when no hook" degraded-mode behavior. + + Mirrors :meth:`WhatsAppAdapter.rehydrate_attachment` exactly (both + platforms are Meta-family and share the same queue-mode failure + shape). Like the original ``_download_attachment``, the rebuilt + closure attaches no auth headers — Messenger payload URLs are + signature-gated by Meta and do not require Bearer tokens. + """ + meta = attachment.fetch_metadata if attachment.fetch_metadata is not None else {} + url = meta.get("url") + if not url: + return attachment + return Attachment( + type=attachment.type, + url=attachment.url, + name=attachment.name, + mime_type=attachment.mime_type, + size=attachment.size, + width=attachment.width, + height=attachment.height, + data=attachment.data, + fetch_data=self._make_attachment_downloader(url), + fetch_metadata=attachment.fetch_metadata, + ) + @staticmethod def _map_attachment_type(fb_type: str) -> str: """Map a Messenger attachment type to the SDK ``Attachment.type`` enum.""" diff --git a/tests/test_messenger_webhook.py b/tests/test_messenger_webhook.py index a7d8e5e..217a72a 100644 --- a/tests/test_messenger_webhook.py +++ b/tests/test_messenger_webhook.py @@ -964,3 +964,117 @@ def _session_get(url: str, **_kw: object) -> _Resp: result = await parsed.attachments[0].fetch_data() assert result == b"image-bytes" + + +# --------------------------------------------------------------------------- +# Attachment rehydration (queue/debounce/burst concurrency) +# --------------------------------------------------------------------------- + + +class TestRehydrateAttachment: + """``MessengerAdapter.rehydrate_attachment`` rebuilds dropped closures. + + Ports the WhatsApp adapter's ``rehydrate_attachment`` hook to Messenger + so that messages bearing attachments survive the queue/debounce/burst + concurrency paths (Codex P2 finding). Without the hook, ``fetch_data`` + is ``None`` after dequeue and downstream handlers cannot download bytes. + """ + + def test_extraction_populates_fetch_metadata_with_url(self) -> None: + """``_extract_attachments`` stores the download URL on ``fetch_metadata``.""" + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.meta", + "text": "x", + "attachments": [ + {"type": "image", "payload": {"url": "https://scontent.example.com/img.jpg"}}, + ], + } + ) + parsed = adapter.parse_message(event) + assert parsed.attachments[0].fetch_metadata == {"url": "https://scontent.example.com/img.jpg"} + + @pytest.mark.asyncio + async def test_rehydrate_rebuilds_fetch_data_after_queue_roundtrip(self) -> None: + """Simulate the queue/serialize cycle and confirm rehydration restores downloads. + + This is the load-bearing test: it would FAIL if ``rehydrate_attachment`` + is left as the protocol default (no-op) because ``fetch_data`` would + stay ``None`` and the ``await`` would raise ``TypeError``. + """ + adapter = _make_adapter() + event = _sample_event( + message={ + "mid": "mid.rehy", + "text": "x", + "attachments": [ + {"type": "image", "payload": {"url": "https://scontent.example.com/img.jpg"}}, + ], + } + ) + parsed = adapter.parse_message(event) + original = parsed.attachments[0] + + # Simulate the JSON roundtrip: ``fetch_data`` closure is dropped, + # ``fetch_metadata`` survives (it serializes as a plain dict). + original.fetch_data = None + + rehydrated = adapter.rehydrate_attachment(original) + assert rehydrated.fetch_data is not None + assert callable(rehydrated.fetch_data) + # Preserves the metadata so a second roundtrip would still work. + assert rehydrated.fetch_metadata == {"url": "https://scontent.example.com/img.jpg"} + + # Wire a fake session to capture the URL the rebuilt closure hits. + captured_urls: list[str] = [] + + class _Resp: + status = 200 + + async def read(self) -> bytes: + return b"rehydrated-bytes" + + async def __aenter__(self) -> _Resp: + return self + + async def __aexit__(self, *_: object) -> None: + pass + + def _session_get(url: str, **_kw: object) -> _Resp: + captured_urls.append(url) + return _Resp() + + mock_session = MagicMock() + mock_session.closed = False + mock_session.get.side_effect = _session_get + adapter._http_session = mock_session + + data = await rehydrated.fetch_data() + assert data == b"rehydrated-bytes" + assert captured_urls == ["https://scontent.example.com/img.jpg"] + + def test_rehydrate_no_metadata_returns_unchanged(self) -> None: + """Degraded mode: attachment without ``fetch_metadata`` is returned as-is.""" + adapter = _make_adapter() + from chat_sdk.types import Attachment + + bare = Attachment(type="image", url="https://example.com/x.jpg") + # fetch_metadata is None by default — adapter cannot rebuild the closure. + result = adapter.rehydrate_attachment(bare) + assert result is bare + assert result.fetch_data is None + + def test_rehydrate_metadata_missing_url_returns_unchanged(self) -> None: + """Degraded mode: ``fetch_metadata`` without the ``url`` key is a no-op.""" + adapter = _make_adapter() + from chat_sdk.types import Attachment + + bare = Attachment( + type="image", + url="https://example.com/x.jpg", + fetch_metadata={"unrelated": "value"}, + ) + result = adapter.rehydrate_attachment(bare) + assert result is bare + assert result.fetch_data is None From 956872a9cab0f0a81e0453f2a63b01d32ff11d2f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 00:49:25 +0000 Subject: [PATCH 5/7] fix(messenger): use is-not-None for limit/user_name/cache checks + regression tests (audit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Targeted audit follow-up — three real truthiness bugs in src/chat_sdk/adapters/messenger/adapter.py, same Port Rule #1 root pattern: 1. _paginate_messages: ``options.limit or 50`` silently swallowed an explicit ``limit=0`` and substituted 50. Switched to ``is not None``. Regression: tests/test_messenger_api.py::test_explicit_zero_limit_is_not_swallowed_to_default. 2. __init__: ``config.user_name or "bot"`` paired with ``bool(config.user_name)`` silently replaced an explicit ``user_name=""`` with the ``"bot"`` fallback AND left ``_has_explicit_user_name`` False, so ``initialize()`` would then overwrite it from ``chat.get_user_name()`` / ``/me``. Switched both sites to ``is not None`` to match upstream's ``hasExplicitUserName`` semantics. Regression: tests/test_messenger_webhook.py::test_explicit_empty_user_name_is_respected. 3. _fetch_user_profile: ``if cached:`` treated a cached ``{}`` (empty dict, falsy) as a miss and re-fetched every call. Switched to ``if cached is not None``. Theoretical (Graph API rarely returns ``{}``) but cheap to make robust. Regression: tests/test_messenger_api.py::test_empty_dict_cache_entry_is_a_hit_not_a_miss. All three regression tests are load-bearing: each asserts an outcome the old code would have produced differently. Other Meta-family adapters (Teams, Discord, Google Chat) already use the ``is not None`` pattern for the same code site. Validation: ruff check / format clean, audit_test_quality.py 0 hard failures, 4235 passed / 3 skipped (was 4232 + 3 new tests = 4235). --- src/chat_sdk/adapters/messenger/adapter.py | 14 +++++--- tests/test_messenger_api.py | 38 ++++++++++++++++++++++ tests/test_messenger_webhook.py | 25 ++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/chat_sdk/adapters/messenger/adapter.py b/src/chat_sdk/adapters/messenger/adapter.py index 8ae7bc2..a05055f 100644 --- a/src/chat_sdk/adapters/messenger/adapter.py +++ b/src/chat_sdk/adapters/messenger/adapter.py @@ -132,9 +132,11 @@ def __init__(self, config: MessengerAdapterConfig) -> None: # If a user_name is provided we treat it as explicit and never # overwrite it from ``/me`` / ``chat.get_user_name()`` — matches the - # upstream ``hasExplicitUserName`` gate. - self._has_explicit_user_name = bool(config.user_name) - self._user_name = config.user_name or "bot" + # upstream ``hasExplicitUserName`` gate. ``is not None`` (not truthy) + # so an explicit ``user_name=""`` is respected rather than silently + # replaced by the ``"bot"`` fallback. + self._has_explicit_user_name = config.user_name is not None + self._user_name = config.user_name if config.user_name is not None else "bot" self._chat: ChatInstance | None = None self._bot_user_id: str | None = None @@ -889,7 +891,9 @@ async def _download_attachment(self, url: str) -> bytes: async def _fetch_user_profile(self, user_id: str) -> MessengerUserProfile: cached = self._user_profile_cache.get(user_id) - if cached: + # ``is not None`` (not truthy): an empty-dict ``{}`` cache entry is + # falsy and would otherwise trigger a re-fetch on every call. + if cached is not None: return cached try: @@ -946,7 +950,7 @@ def _sort_key(message: Message) -> tuple[float, int]: @staticmethod def _paginate_messages(messages: list[Message], options: FetchOptions) -> FetchResult: - limit = max(1, min(options.limit or 50, 100)) + limit = max(1, min(options.limit if options.limit is not None else 50, 100)) direction = options.direction or "backward" if not messages: diff --git a/tests/test_messenger_api.py b/tests/test_messenger_api.py index 4fdac44..60011c2 100644 --- a/tests/test_messenger_api.py +++ b/tests/test_messenger_api.py @@ -451,6 +451,25 @@ async def test_caches_user_profile(self) -> None: # Second call hits the cache — no extra API call. assert adapter._graph_api_fetch.call_count == 1 + @pytest.mark.asyncio + async def test_empty_dict_cache_entry_is_a_hit_not_a_miss(self) -> None: + """Regression: a cached ``{}`` profile must NOT trigger a re-fetch. + + The old ``if cached:`` treated ``{}`` (falsy) as a miss and called the + Graph API every time. The fix ``if cached is not None`` honors the + cache entry regardless of dict contents. Without the fix this test + would observe ``call_count == 1`` (the re-fetch). + """ + adapter = _make_adapter() + adapter._user_profile_cache[RECIPIENT_ID] = {} # pre-populated empty hit + adapter._graph_api_fetch = AsyncMock(return_value={"id": RECIPIENT_ID, "first_name": "Should-Not-Be-Called"}) + + profile = await adapter._fetch_user_profile(RECIPIENT_ID) + + # Cache hit returns the empty dict as-is, no Graph API roundtrip. + assert profile == {} + assert adapter._graph_api_fetch.call_count == 0 + @pytest.mark.asyncio async def test_non_dict_profile_response_falls_back(self) -> None: """A successful Graph API call that returns a non-mapping must not poison the cache. @@ -577,6 +596,25 @@ async def test_clamps_high_limit(self) -> None: result = await adapter.fetch_messages(THREAD_ID, FetchOptions(limit=500)) assert len(result.messages) == 5 + @pytest.mark.asyncio + async def test_explicit_zero_limit_is_not_swallowed_to_default(self) -> None: + """Regression: ``FetchOptions(limit=0)`` must not silently become 50. + + The old ``limit = options.limit or 50`` treated ``0`` as falsy and + substituted the default page size — a caller asking for zero messages + got fifty. Switching to ``is not None`` preserves the explicit value + (then ``max(1, ...)`` clamps to 1, matching ``limit=-N`` behavior). + Without the fix, with 5 messages seeded, this test would observe + ``len(result.messages) == 5`` (all of them, since 50 > 5). + """ + from chat_sdk.types import FetchOptions + + adapter = _make_adapter() + _seed_messages(adapter, 5) + result = await adapter.fetch_messages(THREAD_ID, FetchOptions(limit=0)) + # New behavior: clamped to 1 via ``max(1, min(0, 100))``, NOT 50. + assert len(result.messages) == 1 + @pytest.mark.asyncio async def test_unknown_cursor_backward(self) -> None: from chat_sdk.types import FetchOptions diff --git a/tests/test_messenger_webhook.py b/tests/test_messenger_webhook.py index 217a72a..0f874cd 100644 --- a/tests/test_messenger_webhook.py +++ b/tests/test_messenger_webhook.py @@ -199,6 +199,31 @@ def test_constructor_also_raises_on_missing_credentials(self, _clear_messenger_e ) ) + def test_explicit_empty_user_name_is_respected(self, _clear_messenger_env: Any) -> None: + """Regression: ``user_name=""`` must be treated as an explicit choice. + + The old ``config.user_name or "bot"`` (paired with + ``bool(config.user_name)``) silently replaced an explicit empty + string with the ``"bot"`` default and left ``_has_explicit_user_name`` + ``False`` — so ``initialize()`` would then overwrite it from + ``chat.get_user_name()`` / ``/me``. Switching both sites to + ``is not None`` honors the explicit empty string. + + Without the fix this test would observe ``_user_name == "bot"`` and + ``_has_explicit_user_name is False``. + """ + adapter = MessengerAdapter( + MessengerAdapterConfig( + app_secret=APP_SECRET, + page_access_token=PAGE_TOKEN, + verify_token=VERIFY_TOKEN, + user_name="", + logger=ConsoleLogger("error"), + ) + ) + assert adapter._user_name == "" + assert adapter._has_explicit_user_name is True + # --------------------------------------------------------------------------- # Thread ID encoding From cdb38809b74a621c60e97c15d2a292aefd1c6a05 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 02:19:58 +0000 Subject: [PATCH 6/7] fix(messenger): thread echo events by recipient PSID in parse_message (codex review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a Messenger echo event (a bot-sent message echoed back), sender.id is the Page ID and recipient.id is the user's PSID — the reverse of a normal inbound message. parse_message previously keyed the thread ID off sender.id unconditionally, so a replayed echo was threaded under messenger: instead of messenger:, causing fetch_messages('messenger:') to miss the bot's own echoed message. Branch on message.is_echo so the thread ID keys off recipient.id for echoes (matching _handle_echo) and sender.id otherwise. Non-echo behavior unchanged. https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj --- src/chat_sdk/adapters/messenger/adapter.py | 11 +++++- tests/test_messenger_api.py | 39 +++++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/chat_sdk/adapters/messenger/adapter.py b/src/chat_sdk/adapters/messenger/adapter.py index a05055f..9d30434 100644 --- a/src/chat_sdk/adapters/messenger/adapter.py +++ b/src/chat_sdk/adapters/messenger/adapter.py @@ -735,7 +735,16 @@ def _resolve_thread_id(self, value: str) -> MessengerThreadId: def parse_message(self, raw: MessengerRawMessage) -> Message: """Parse a raw messaging event into a normalized ``Message``.""" sender = raw.get("sender") or {} - thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=sender.get("id", ""))) + message_data = raw.get("message") or {} + # For echoes (bot-sent messages echoed back) ``sender.id`` is the Page + # ID and ``recipient.id`` is the user's PSID — the reverse of a normal + # inbound event. Thread off the user PSID so the message threads under + # the same conversation ``_handle_echo`` uses. + if message_data.get("is_echo"): + recipient = raw.get("recipient") or {} + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=recipient.get("id", ""))) + else: + thread_id = self.encode_thread_id(MessengerThreadId(recipient_id=sender.get("id", ""))) message = self._parse_messenger_message(raw, thread_id) self._cache_message(message) return message diff --git a/tests/test_messenger_api.py b/tests/test_messenger_api.py index 60011c2..97e7c31 100644 --- a/tests/test_messenger_api.py +++ b/tests/test_messenger_api.py @@ -25,7 +25,7 @@ MESSENGER_MESSAGE_LIMIT, MessengerAdapter, ) -from chat_sdk.adapters.messenger.types import MessengerAdapterConfig +from chat_sdk.adapters.messenger.types import MessengerAdapterConfig, MessengerThreadId from chat_sdk.logger import ConsoleLogger from chat_sdk.shared.errors import ( AdapterRateLimitError, @@ -733,6 +733,43 @@ def test_echo_marks_as_me_and_bot(self) -> None: assert parsed.author.is_me is True assert parsed.author.is_bot is True + def test_echo_threads_by_recipient_psid(self) -> None: + """Echo events flip sender/recipient: ``sender.id`` is the Page ID and + ``recipient.id`` is the user's PSID. ``parse_message`` must thread the + echo off the user PSID so it lands in the same conversation as the + inbound user messages (matching ``_handle_echo``), not the page id. + """ + adapter = _make_adapter() + adapter._bot_user_id = "PAGE_456" + event = { + "sender": {"id": "PAGE_456"}, + "recipient": {"id": "USER_123"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.echo", "text": "bot says", "is_echo": True}, + } + parsed = adapter.parse_message(event) + # Threaded under the user's PSID, NOT the page id. + assert parsed.thread_id == "messenger:USER_123" + assert parsed.thread_id != "messenger:PAGE_456" + # Must match what _handle_echo derives for the same event. + expected = adapter.encode_thread_id(MessengerThreadId(recipient_id="USER_123")) + assert parsed.thread_id == expected + + def test_non_echo_threads_by_sender_psid(self) -> None: + """Non-echo inbound messages have ``sender.id`` == user PSID and must + keep threading off the sender — guards against over-applying the echo + fix to normal inbound events. + """ + adapter = _make_adapter() + event = { + "sender": {"id": "USER_123"}, + "recipient": {"id": "PAGE_456"}, + "timestamp": 1735689600000, + "message": {"mid": "mid.abc", "text": "hi"}, + } + parsed = adapter.parse_message(event) + assert parsed.thread_id == "messenger:USER_123" + def test_postback_uses_title_as_text(self) -> None: adapter = _make_adapter() event = { From 14052713c1ce4bfe133210002aa01b674a7da148 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 30 May 2026 02:25:32 +0000 Subject: [PATCH 7/7] build(messenger): add messenger extra declaring aiohttp (codex review) The Messenger adapter lazy-imports aiohttp on its runtime paths (initialize, send, profile lookup, attachment downloads), but the package ships `dependencies = []` and had no `messenger` optional-dependencies extra. A base install enabling only Messenger would raise `ModuleNotFoundError: aiohttp` the moment the adapter talks to Meta. Add `messenger = ["aiohttp>=3.9"]`, matching the sibling aiohttp adapters (telegram/whatsapp/teams/linear). The `all` extra already includes aiohttp>=3.9, so no change there. https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 6d5e661..c8459f7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,7 @@ discord = ["pynacl>=1.5", "aiohttp>=3.9"] teams = ["aiohttp>=3.9"] telegram = ["aiohttp>=3.9"] whatsapp = ["aiohttp>=3.9"] +messenger = ["aiohttp>=3.9"] google-chat = ["aiohttp>=3.9", "pyjwt[crypto]>=2.8", "google-auth>=2.0"] linear = ["aiohttp>=3.9"] all = [