From a46f28b46df22b7a1a1f8f080c7d53d7f5e4f921 Mon Sep 17 00:00:00 2001 From: patrick-chinchill Date: Wed, 30 Sep 2026 23:51:05 -0700 Subject: [PATCH 1/2] feat(core): Thread.reply, Thread.mark_as_read, post_ephemeral options (#200) Ports vercel/chat 83ede7ea (#819), 18d4a230 (#820), the core doc part of 160140e3 (#737) and the core slice of bfee00af (#939): - Thread.reply(target, message): native replies with reply_to, known-message resolution (never fetched), stream buffering, history append - Thread.mark_as_read(message=None) with upstream error order - optional BaseAdapter.reply / mark_as_read hooks; Thread Protocol members - post_ephemeral forwards options= (signature probe keeps 3-arg custom adapters working) and may return None - SentMessage.edit keeps the resolved thread_id and reply_to - mock adapter records mark_as_read by default Ports all 16 [markAsRead]/[reply()] upstream tests. --- CHANGELOG.md | 10 + docs/UPSTREAM_SYNC.md | 70 ++++ scripts/fidelity_target.json | 36 +- src/chat_sdk/_compat.py | 54 +++ src/chat_sdk/adapters/google_chat/adapter.py | 9 +- src/chat_sdk/adapters/slack/adapter.py | 9 +- src/chat_sdk/channel.py | 10 +- src/chat_sdk/shared/mock_adapter.py | 7 + src/chat_sdk/thread.py | 132 +++++- src/chat_sdk/types.py | 69 +++- tests/test_channel_faithful.py | 28 +- tests/test_compat.py | 104 +++++ tests/test_thread_faithful.py | 399 ++++++++++++++++++- 13 files changed, 894 insertions(+), 43 deletions(-) create mode 100644 src/chat_sdk/_compat.py create mode 100644 tests/test_compat.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8f2382..f9f078e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ Sync wave from `chat@4.31.0` to `chat@4.41.1` (tracking #184). `UPSTREAM_PARITY` stays `4.31.0` until #203. +- **Core: `Thread.reply`, `Thread.mark_as_read`, `post_ephemeral` options** (#200). Ports vercel/chat `83ede7ea` (#819) and `18d4a230` (#820) from chat@4.38.0, the doc-only core part of `160140e3` (#737, chat@4.35.0) and the core slice of `bfee00af` (#939, chat@4.41.0). Core API only; adapter support comes in #228 (Telegram), #239 (WhatsApp/Messenger), #219 (Teams) and #241 (Gmail). + - New `await thread.reply(target, message)` posts a native reply to a `Message` or message ID. A `Message` from another thread raises `ChatError`. A string ID is resolved only against the current message and `recent_messages`, and is never fetched. Streams are buffered into one markdown reply (`" "` if they produce no text). The returned `SentMessage.reply_to` is the target, and the reply is appended to thread history. + - New `await thread.mark_as_read(message=None)` sends a read receipt. It defaults to the message being handled. With no message available it raises `ChatError("A message is required outside a message handler")`, and for a `Message` from another thread it raises `ChatError("Cannot mark a message from another thread as read")`. + - Optional adapter hooks `reply(thread_id, message_id, message) -> RawMessage` and `mark_as_read(thread_id, message_id, message=None)` are declared on `BaseAdapter` (not on the `Adapter` Protocol). Both are also on the `Thread` Protocol. + - **Consumer-visible:** `thread.reply()` / `thread.mark_as_read()` raise `ChatNotImplementedError` (`"replies"` / `"read-receipts"`) on adapters without the hook. No in-repo adapter has `reply` until #228/#239. Until #239, `thread.mark_as_read()` on WhatsApp raises `TypeError`, because `WhatsAppAdapter.mark_as_read(message_id)` keeps its one-argument signature. + - **Consumer-visible:** `Thread.post_ephemeral` / `Channel.post_ephemeral` now pass the caller's `PostEphemeralOptions` to the adapter as `options=`. An adapter may return `None` (no private delivery path), which is returned unchanged with no DM fallback, as upstream. `BaseAdapter.post_ephemeral` is now `(thread_id, user_id, message, *, options=None) -> EphemeralMessage | None`. Slack and Google Chat accept `options` and ignore it. Tests that assert the adapter call with three arguments need `options=...` added. + - **Fix:** `SentMessage.edit()` keeps the thread ID the adapter returned for the original post (it used to fall back to the thread's own ID) and keeps `reply_to`, as upstream. + - `chat_sdk.testing.create_mock_adapter()` has a recording `mark_as_read` `AsyncMock` by default, as upstream's mock does. Set `adapter.mark_as_read = None` to test the unsupported path. + - **Python-specific (divergence from upstream):** `options=` reaches only `post_ephemeral` implementations that accept it (an `options` parameter or `**kwargs`). Custom adapters written against the older 3-argument signature keep working instead of raising `TypeError`. See `docs/UPSTREAM_SYNC.md`. + - Fidelity: all 16 `[markAsRead]` / `[reply()]` tests ported under their exact names; `thread.test.ts` 15 ā†’ 1 missing at `chat@4.41.1`. - **Slack: message edits and deletes reach `on_message_updated` / `on_message_deleted`** (#211; Slack half of vercel/chat `4ac04551` #788, chat@4.37.0, and the `handleMessageChanged` part of `864d9222` #846, chat@4.39.0). - **Consumer-visible (Slack):** a user's edit (`message_changed`) now calls `on_message_updated(thread, message, previous_message)`, where `previous_message` is the pre-edit message when Slack sent one (parsed the same way as the new message, falling back to a sync parse with a warning if the user lookup fails) and `None` otherwise. A delete (`message_deleted`) now calls `on_message_deleted(event)` with `channel_id`, `message_id` (the deleted ts), `thread_id`, `deleted_at` (UTC, from `event_ts`), `previous_message` when Slack sent one, and the raw payload. Edits and deletes are still never routed as new messages (`on_mention` / `on_subscribed_message` / `on_message`). With no handler registered, no handler runs, but each user edit is still parsed and dispatched through core as upstream does (a cached `users.info` lookup, a thread-participant state write, up to 2 s of unfurl polling when the text has links, and the subscription/identity state reads); previously a `message_changed` without unfurls did no I/O. Each delete builds a sync-parsed `MessageDeletedEvent` (no lookup). - Not reported as edits: unfurl-only updates (unfurl metadata is still cached), hidden thread metadata updates, Slack language-detection updates with no content change, inner `tombstone` replacements, and the bot's own edits (post+edit and native streaming). diff --git a/docs/UPSTREAM_SYNC.md b/docs/UPSTREAM_SYNC.md index bb13c0cc..2e8b8081 100644 --- a/docs/UPSTREAM_SYNC.md +++ b/docs/UPSTREAM_SYNC.md @@ -1773,6 +1773,75 @@ from `21dc60c3` (#935, chat@4.41.0), and the `messages.test.ts` case of asserting the same `data:image/png;base64,AQID` part. As before, an unnamed attachment gets `filename=""` (upstream leaves it `undefined`). +### Thread replies, read receipts and ephemeral options (chat@4.35ā€“4.41, #200) + +Parity with `83ede7ea` (vercel/chat#819) and `18d4a230` (#820, chat@4.38.0), +the doc-only core part of `160140e3` (#737, chat@4.35.0), and the core slice +of `bfee00af` (#939, chat@4.41.0). One divergence-table row (the +`post_ephemeral` signature probe). Adapter implementations come later: +Telegram replies #228, WhatsApp/Messenger reply and read receipts #239, Teams +targeted ephemeral #219, Gmail #241. + +- **`Thread.reply(target, message)` (`83ede7ea`).** Steps, in upstream + order: check for the hook, resolve the target, check the thread, buffer a + stream, process callback URLs, call `adapter.reply(thread_id, + target_id, postable)`, build the `SentMessage` with `reply_to`, append it + to thread history. A string target is resolved by + `_find_known_message` against the current message and then + `recent_messages`. It is never fetched. An unknown ID is still sent, with + `reply_to=None` and no cross-thread check. Streams go through + `from_full_stream`. Text and `markdown_text` chunks are kept and + `task_update` / `plan_update` chunks are dropped. A stream with no visible + text posts `PostableMarkdown(" ")`, as upstream. Python has no JSX, so a + `CardElement` dict is passed through, and the upstream JSX test checks that. +- **`Thread.mark_as_read(message=None)` (`18d4a230`).** Errors come in + upstream order: no hook, then `MESSAGE_REQUIRED`, then `THREAD_MISMATCH`. + `None` means the current message, checked with `is None`. An explicit `""` + is rejected as `MESSAGE_REQUIRED`, as upstream (`"" ?? current` keeps `""`, + then `!target` throws). The adapter receives + `(thread_id, message_id, message_or_None)`. +- **Optional hooks.** `reply` and `mark_as_read` are declared on + `BaseAdapter` with defaults that raise `ChatNotImplementedError` with + `"replies"` / `"read-receipts"`, the same features `Thread` raises. They + are not on the `Adapter` Protocol, where they would become required. + `Thread` finds the hooks with `getattr(adapter, name, None)`. A + `BaseAdapter` subclass without an override therefore reaches the default, + which raises the same error after buffering and callback processing. + The shared mock adapter has a recording `mark_as_read` `AsyncMock` by + default, as upstream's mock does, and no `reply`. +- **Errors.** Python's `ChatError` has no `code`, so `MESSAGE_REQUIRED` and + `THREAD_MISMATCH` are `ChatError`s with upstream's exact messages. + Upstream's cross-thread reply check throws a plain `Error`. Python raises + `ChatError` with the same message, as the History API does (#197). No + `code` attribute is added. `NotImplementedError.feature` is + `ChatNotImplementedError.method`. +- **`SentMessage.edit()` keeps the thread ID and target.** `_create_sent_message` + takes `reply_to`. Its `edit` now returns + `_create_sent_message(id, content, thread_id, reply_to=reply_to)`, as + upstream's `createSentMessage(messageId, postable, threadId, replyTo)` + does. Before this change, an edit of a message posted under a + `thread_id_override` went back to `self._id`. `create_sent_message_from_message` + passes `message.reply_to` the same way. +- **`post_ephemeral` options (`bfee00af`).** `BaseAdapter.post_ephemeral` + is now `(thread_id, user_id, message, *, options=None) -> EphemeralMessage + | None`. `Thread` and `Channel` pass the caller's `PostEphemeralOptions` + as `options=`, and an adapter's `None` comes back unchanged with no DM + fallback, as upstream. Slack and Google Chat accept `options` and ignore + it. JavaScript drops extra arguments; Python raises `TypeError`. So + `options=` is passed only when `chat_sdk._compat.accepts_kwarg` reports + that the implementation takes it: a parameter named `options` that is + positional-or-keyword or keyword-only, or `**kwargs`. The check is cached + per function, and bound methods are checked through `__func__`. A callable + that cannot be introspected gets `options=`, as upstream always passes it. + This is the divergence row. #201 (`start_typing(options=)`) can reuse it. +- **WhatsApp.** Until #239, `WhatsAppAdapter.mark_as_read(message_id)` keeps + its one-argument signature, so `thread.mark_as_read()` on WhatsApp raises + `TypeError`. +- **Fidelity.** All 16 `[markAsRead]` / `[reply()]` tests are ported under + their exact names: `thread.test.ts` 15 ā†’ 1 missing at `chat@4.41.1`. The + remaining test, `startTyping`, belongs to #201. The `postEphemeral` + assertions in `thread.test.ts` / `channel.test.ts` now expect `options=`. + ## What to Port vs What to Adapt ### Port 1:1 @@ -2194,6 +2263,7 @@ stay explicit instead of being rediscovered in code review. | Channel edit callback scope (4.41 wave, #194) | A channel `SentMessage.edit` binds new callback tokens to `{channel.id, "channel"}`, the scope the original `channel.post` used | `createSentMessage(...).edit` binds to `{threadId, "thread"}`, where `threadId` is the id the adapter reported for the post (or the channel id) | The reported thread id often never equals a click's thread id: Teams and Google Chat report the channel id (upstream as well; Teams clicks carry `;messageid=`, Google Chat clicks carry the thread name), Python's Slack reports the synthetic `slack:Cā€¦:` until #283 while clicks carry the message ts, a Slack DM click carries no ts even after #283 makes the post report one, and a chained edit dropped the override until #195 ported `16ea171e`. Upstream's edited buttons never POST in those cases. Every click on the message derives the channel id, and the channel scope is no broader than the original post's. Regression tests: `tests/test_channel_faithful.py::TestCallbackUrlProcessing::test_edited_slack_channel_card_resolves_for_the_real_click` (real Slack id functions and `_handle_block_actions`, channel and DM, before and after #283), `::test_edited_teams_channel_card_resolves_for_a_click_in_that_channel` (real Teams id functions) and `::test_chained_edit_keeps_callback_tokens_resolvable`. To be filed as an upstream issue against vercel/chat (Teams and Google Chat edited channel cards never POST); upstream Slack is unaffected, since its `postChannelMessage` and DM clicks both carry the message ts. | | Callback-token lease fence (4.41 wave, #194) | After deleting a matched record, `resolve_callback_url` calls `extend_lock(lock, CALLBACK_LOCK_TTL_MS)`. If that fails, the 10 s lease lapsed mid-consume, and the call returns `None` instead of the record (fail closed: no POST, raw `__cb:` value to handlers) | `resolveCallbackUrl` returns the record after `delete` regardless of lease state, so if a `get`/`delete` stalls past the lease, a second click that takes the expired lock also resolves it and both POST | Keeps the single-use contract under state-backend stalls. `extend_lock` checks token ownership and expiry in every backend (Memory, Redis script, Postgres `WHERE token = $4 AND expires_at > now()`), and `Chat` already relies on it for lock heartbeats. The cost is one extra state call per resolved click. A stalled consume that loses its lease also burns the token without a POST. To be filed as an upstream issue against vercel/chat (a stalled `get`/`delete` past the 10 s lease lets a second click double-POST). Regression test: `tests/test_callback_url.py::TestResolveCallbackUrlLocking::test_lost_lease_fails_closed_instead_of_double_consuming`. | | Link-preview fence slicing (4.41 wave, #195) | `_render_link_for_prompt` bounds url/title/description/site with Python slicing, which counts code points | `renderLinkForPrompt` slices with `String.prototype.slice`, which counts UTF-16 code units | Only differs for astral characters (emoji and the like): a bounded field keeps up to the limit in code points, where JS keeps half as many astral characters and can end on a lone surrogate. Emulating UTF-16 slicing would produce lone surrogates that break UTF-8 encoding of the prompt. Whitespace handling is not a divergence: the normalizer uses JS's exact `\s`/`trim` set. Regression test: `tests/test_ai_messages.py::TestLinkPreviews::test_link_metadata_bounds_count_code_points_not_utf16_units`. | +| `post_ephemeral` signature probe (4.41 wave, #200) | `Thread`/`Channel.post_ephemeral` pass `options=` to `adapter.post_ephemeral` only when `chat_sdk._compat.accepts_kwarg` finds an `options` parameter (positional-or-keyword or keyword-only) or `**kwargs`; an implementation with the older `(thread_id, user_id, message)` signature is called with three arguments | `adapter.postEphemeral(threadId, userId, postable, options)` always | JavaScript drops extra arguments, while Python raises `TypeError`, so passing `options=` unconditionally would break third-party adapters written against the pre-4.41 signature. In-repo adapters all accept it. Regression tests: `tests/test_thread_faithful.py::TestPostEphemeral::test_three_argument_custom_post_ephemeral_still_works`, `tests/test_channel_faithful.py::TestChannelPostEphemeral::test_three_argument_custom_post_ephemeral_still_works_from_a_channel` and `tests/test_compat.py::test_accepts_kwarg`. | | Markdown parser | Subset of CommonMark (no setext headings, indented code, HTML, escaped chars, backtick spans >1) | Full CommonMark via remark | See [DECISIONS.md](DECISIONS.md#why-hand-rolled-markdown-parser) | | `_remend` streaming repair | Parity-based emphasis closing | `remend` npm package | Simplified; handles common cases | | `walkAst` | Deep-copies the tree (immutable) | Mutates the tree in place | Python convention; safer | diff --git a/scripts/fidelity_target.json b/scripts/fidelity_target.json index 5dcfbbda..a6b8952b 100644 --- a/scripts/fidelity_target.json +++ b/scripts/fidelity_target.json @@ -5,10 +5,10 @@ "totals": { "ts_tests": 1064, "each_templates": 21, - "matched_exact": 869, - "matched_fuzzy": 77, - "missing": 118, - "extra": 514 + "matched_exact": 885, + "matched_fuzzy": 75, + "missing": 104, + "extra": 526 }, "absent_ts_files": [], "files": { @@ -45,34 +45,18 @@ "python_exists": true, "ts_tests": 159, "each_templates": 1, - "matched_exact": 137, - "matched_fuzzy": 7, - "missing_count": 15, - "extra_count": 30, + "matched_exact": 153, + "matched_fuzzy": 5, + "missing_count": 1, + "extra_count": 41, "missing": [ - ["startTyping", "passes the initiating user and clears processing after posting"], - ["markAsRead", "marks the current message when no target is provided"], - ["markAsRead", "marks an explicit message id"], - ["markAsRead", "marks an explicit message"], - ["markAsRead", "rejects a message from another thread"], - ["markAsRead", "requires a target outside a message handler"], - ["markAsRead", "throws when the adapter does not support read receipts"], - ["markAsRead", "preserves the current message through serialization"], - ["reply()", "throws when the adapter does not support replies"], - ["reply()", "accepts a Message and preserves it on the result"], - ["reply()", "rejects a Message from another thread"], - ["reply()", "converts JSX cards before delegating"], - ["reply()", "buffers streams into one markdown reply"], - ["reply()", "resolves a message id against messages the thread knows"], - ["reply()", "leaves replyTo undefined for an unknown message id"] + ["startTyping", "passes the initiating user and clears processing after posting"] ], "fuzzy_matches": [ ["should pass stream options from Slack current message context via $label", "test_should_pass_stream_options_from_current_message_context"], ["should forward structured stream chunks to adapter.stream from an action-created thread", "test_should_forward_structured_stream_chunks_to_adapter_stream_from_an_action_created_thread"], ["should still update last in_progress task when no ID provided", "test_should_still_update_last_in_progress_task_when_no_id_provided"], ["should not error when adapter has no onThreadSubscribe", "test_not_error_when_no_on_thread_subscribe"], - ["delegates a message id and content to the adapter", "test_sent_message_reflects_stranded_placeholder_on_strict_adapter"], - ["falls back to a space when a stream produces no text", "test_should_clear_placeholder_when_stream_is_whitespace_only"], ["should resolve a JSX Card and its children before scheduling", "test_should_convert_card_jsx_with_children_to_cardelement"] ] }, @@ -85,7 +69,7 @@ "matched_exact": 66, "matched_fuzzy": 1, "missing_count": 0, - "extra_count": 9, + "extra_count": 10, "missing": [], "fuzzy_matches": [ ["should resolve a JSX Card and its children before scheduling", "test_should_convert_jsx_card_elements_to_cardelement"] diff --git a/src/chat_sdk/_compat.py b/src/chat_sdk/_compat.py new file mode 100644 index 00000000..509df026 --- /dev/null +++ b/src/chat_sdk/_compat.py @@ -0,0 +1,54 @@ +"""Python-only compatibility helpers for calling adapter hooks. + +Upstream (TypeScript) can add a trailing optional parameter to an adapter +hook and every existing implementation keeps working, because JavaScript +silently drops extra arguments. Python raises ``TypeError`` instead, so when +the SDK starts passing a new keyword to a hook (``post_ephemeral(options=)``) +it first checks that the implementation accepts it. In-repo adapters always +do; the probe keeps third-party adapters written against the older signature +working. See docs/UPSTREAM_SYNC.md (Known Non-Parity). +""" + +from __future__ import annotations + +import functools +import inspect +from collections.abc import Callable +from typing import Any + + +@functools.lru_cache(maxsize=512) +def _accepts_kwarg_cached(func: Callable[..., Any], name: str) -> bool: + return _accepts_kwarg_uncached(func, name) + + +def _accepts_kwarg_uncached(func: Callable[..., Any], name: str) -> bool: + try: + params = inspect.signature(func).parameters.values() + except (TypeError, ValueError): + # Not introspectable (some builtins / C callables): pass the keyword, + # as upstream always does. + return True + for param in params: + if param.kind is inspect.Parameter.VAR_KEYWORD: + return True + if param.name == name and param.kind in ( + inspect.Parameter.POSITIONAL_OR_KEYWORD, + inspect.Parameter.KEYWORD_ONLY, + ): + return True + return False + + +def accepts_kwarg(func: Callable[..., Any], name: str) -> bool: + """Return whether ``func`` can be called with keyword argument ``name``. + + ``**kwargs`` counts as accepting. Bound methods are probed through their + underlying function so the result is cached per class, not per instance. + """ + target = getattr(func, "__func__", func) + try: + return _accepts_kwarg_cached(target, name) + except TypeError: + # Unhashable callable: probe without the cache. + return _accepts_kwarg_uncached(target, name) diff --git a/src/chat_sdk/adapters/google_chat/adapter.py b/src/chat_sdk/adapters/google_chat/adapter.py index dc73f3bf..e7041122 100644 --- a/src/chat_sdk/adapters/google_chat/adapter.py +++ b/src/chat_sdk/adapters/google_chat/adapter.py @@ -76,6 +76,7 @@ LockScope, Message, PostableMarkdown, + PostEphemeralOptions, RawMessage, ReactionEvent, StateAdapter, @@ -1861,8 +1862,14 @@ async def post_ephemeral( thread_id: str, user_id: str, message: AdapterPostableMessage, + *, + options: PostEphemeralOptions | None = None, ) -> EphemeralMessage: - """Post an ephemeral (user-only visible) message.""" + """Post an ephemeral (user-only visible) message. + + ``options`` is accepted for the adapter contract and unused: delivery + is always native. + """ decoded = self.decode_thread_id(thread_id) space_name = decoded.space_name thread_name = decoded.thread_name diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index 6b147886..7b0cee04 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -123,6 +123,7 @@ ModalSubmitEvent, OptionsLoadEvent, PostableMarkdown, + PostEphemeralOptions, RawMessage, ReactionEvent, ScheduledMessage, @@ -4902,8 +4903,14 @@ async def post_ephemeral( thread_id: str, user_id: str, message: AdapterPostableMessage, + *, + options: PostEphemeralOptions | None = None, ) -> EphemeralMessage: - """Post an ephemeral (user-only visible) message.""" + """Post an ephemeral (user-only visible) message. + + ``options`` is accepted for the adapter contract and unused: delivery + is always native. + """ message = await self._resolve_message_mentions(message, thread_id) decoded = self.decode_thread_id(thread_id) channel = decoded.channel diff --git a/src/chat_sdk/channel.py b/src/chat_sdk/channel.py index 5314a58d..bf169810 100644 --- a/src/chat_sdk/channel.py +++ b/src/chat_sdk/channel.py @@ -12,6 +12,7 @@ from datetime import datetime, timezone from typing import Any +from chat_sdk._compat import accepts_kwarg from chat_sdk.callback_url import CallbackScope, process_card_callback_urls from chat_sdk.errors import ChatNotImplementedError from chat_sdk.plan import is_postable_object, post_postable_object @@ -351,9 +352,14 @@ async def post_ephemeral( # Callback tokens are minted per delivery path so each is bound to # the conversation the card actually lands in (vercel/chat#875). - if hasattr(self.adapter, "post_ephemeral") and self.adapter.post_ephemeral: # type: ignore[union-attr] + native = getattr(self.adapter, "post_ephemeral", None) + if native: message = await self._process_callback_urls(message) # type: ignore[assignment] - return await self.adapter.post_ephemeral(self._id, user_id, message) # type: ignore[union-attr] + # ``options=`` only reaches implementations that accept it + # (Python-only compatibility probe; see chat_sdk._compat). + if accepts_kwarg(native, "options"): + return await native(self._id, user_id, message, options=options) + return await native(self._id, user_id, message) if not options.fallback_to_dm: return None diff --git a/src/chat_sdk/shared/mock_adapter.py b/src/chat_sdk/shared/mock_adapter.py index e7671d04..d5cefcaa 100644 --- a/src/chat_sdk/shared/mock_adapter.py +++ b/src/chat_sdk/shared/mock_adapter.py @@ -100,6 +100,13 @@ def __init__(self, name: str = "slack") -> None: self._fetch_calls: list[tuple[str, FetchOptions | None]] = [] self._initialize_calls: list[Any] = [] + # Read receipts are enabled by default, as in upstream's mock + # (``markAsRead: vi.fn()``); tests that need an adapter without them + # set ``adapter.mark_as_read = None``. ``reply`` is absent by default. + from unittest.mock import AsyncMock + + self.mark_as_read: Any = AsyncMock(name="mark_as_read", return_value=None) + async def initialize(self, chat: Any) -> None: self._initialize_calls.append(chat) diff --git a/src/chat_sdk/thread.py b/src/chat_sdk/thread.py index 1ff208e6..374a3d70 100644 --- a/src/chat_sdk/thread.py +++ b/src/chat_sdk/thread.py @@ -9,13 +9,14 @@ import asyncio import contextvars -from collections.abc import AsyncGenerator, AsyncIterator +from collections.abc import AsyncGenerator, AsyncIterable, AsyncIterator from dataclasses import dataclass, replace from datetime import datetime, timezone from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable +from chat_sdk._compat import accepts_kwarg from chat_sdk.callback_url import CallbackScope, process_card_callback_urls -from chat_sdk.errors import ChatNotImplementedError +from chat_sdk.errors import ChatError, ChatNotImplementedError from chat_sdk.from_full_stream import from_full_stream from chat_sdk.logger import Logger from chat_sdk.plan import is_postable_object, post_postable_object @@ -702,9 +703,17 @@ async def post_ephemeral( # Callback tokens are minted per delivery path so each is bound to # the conversation the card actually lands in (vercel/chat#875). # Try native ephemeral - if hasattr(self.adapter, "post_ephemeral") and self.adapter.post_ephemeral: # type: ignore[union-attr] + native = getattr(self.adapter, "post_ephemeral", None) + if native: message = await self._process_callback_urls(message) # type: ignore[assignment] - return await self.adapter.post_ephemeral(self._id, user_id, message) # type: ignore[union-attr] + # The adapter may return ``None`` (no private delivery path); that + # is returned unchanged, with no DM fallback, as upstream does. + # Python-only: ``options=`` is passed only to implementations that + # accept it, so adapters written against the older 3-argument + # signature keep working (see chat_sdk._compat). + if accepts_kwarg(native, "options"): + return await native(self._id, user_id, message, options=options) + return await native(self._id, user_id, message) if not options.fallback_to_dm: return None @@ -726,6 +735,81 @@ async def post_ephemeral( return None + async def reply( + self, + target: str | Message, + message: AdapterPostableMessage | AsyncIterable[Any], + ) -> SentMessage: + """Reply to a specific message in this thread with the platform's native reply. + + Raises :class:`~chat_sdk.errors.ChatNotImplementedError` (``"replies"``) + on adapters without a ``reply`` hook. + + ``target`` is the message to reply to, or its ID. Prefer passing the + :class:`Message`: it is checked against this thread and carried through + to ``SentMessage.reply_to`` and cached thread history. A raw ID is + resolved only against messages this thread already holds + (``recent_messages`` or the message being handled) and is never + fetched; an unknown ID is still sent, with ``reply_to`` left ``None`` + and no cross-thread check. Streams are buffered and posted as one + markdown message rather than streamed edit-by-edit. + """ + reply_hook = getattr(self.adapter, "reply", None) + if not reply_hook: + raise ChatNotImplementedError(self.adapter.name, "replies") + + target_message = target if isinstance(target, Message) else self._find_known_message(target) + + if target_message is not None and target_message.thread_id != self._id: + # Upstream throws a plain ``Error``; ``ChatError`` keeps the SDK's + # error hierarchy (same message). See docs/UPSTREAM_SYNC.md. + raise ChatError("Cannot reply to a message from another thread") + + postable: AdapterPostableMessage + if _is_async_iterable(message): + markdown = "" + async for chunk in from_full_stream(message): # type: ignore[arg-type] + if isinstance(chunk, str): + markdown += chunk + elif getattr(chunk, "type", None) == "markdown_text": + # Only MarkdownTextChunk carries ``.text`` (pyrefly does + # not narrow on the ``type`` tag). + markdown += getattr(chunk, "text", "") + elif isinstance(chunk, dict) and chunk.get("type") == "markdown_text": + markdown += chunk.get("text", "") + # A stream can finish without producing text (tool calls only, or + # just task_update/plan_update chunks). Platforms reject empty + # bodies, so fall back to a single space as upstream does. + postable = PostableMarkdown(markdown=markdown if markdown.strip() else " ") + else: + postable = message # type: ignore[assignment] + + postable = await self._process_callback_urls(postable) + target_id = target if isinstance(target, str) else target.id + raw_msg: RawMessage = await reply_hook(self._id, target_id, postable) + result = self._create_sent_message( + raw_msg.id, + postable, + raw_msg.thread_id, + raw=raw_msg.raw, + reply_to=target_message, + ) + + if self._thread_history is not None: + await self._thread_history.append(self._id, _to_message(result)) + + return result + + def _find_known_message(self, message_id: str) -> Message | None: + """Resolve a message ID against messages this thread already holds. + + Deliberately does not fetch: callers that need ``reply_to`` populated + for certain should pass the :class:`Message` itself. + """ + if self._current_message is not None and self._current_message.id == message_id: + return self._current_message + return next((m for m in self._recent_messages if m.id == message_id), None) + async def _process_callback_urls( self, postable: str | AdapterPostableMessage, @@ -1110,6 +1194,37 @@ async def _edit_loop() -> None: async def start_typing(self, status: str | None = None) -> None: await self.adapter.start_typing(self._id, status) + async def mark_as_read(self, message: str | Message | None = None) -> None: + """Send a read receipt for an inbound message. + + Defaults to the message being handled, so it takes no argument inside + a message handler. Pass a :class:`Message` or a message ID to target + one explicitly; a ``Message`` must belong to this thread. Platforms + may treat the receipt as a watermark and mark earlier messages read + along with the target. + + Raises :class:`~chat_sdk.errors.ChatNotImplementedError` + (``"read-receipts"``) on adapters without a ``mark_as_read`` hook. + """ + mark_hook = getattr(self.adapter, "mark_as_read", None) + if not mark_hook: + raise ChatNotImplementedError(self.adapter.name, "read-receipts") + + # ``None`` means "the current message"; upstream's ``message ?? + # currentMessage`` followed by ``!target`` also rejects an empty ID. + target = message if message is not None else self._current_message + if target is None or target == "": + raise ChatError("A message is required outside a message handler") + + if isinstance(target, str): + await mark_hook(self._id, target, None) + return + + if target.thread_id != self._id: + raise ChatError("Cannot mark a message from another thread as read") + + await mark_hook(self._id, target.id, target) + # -- Refresh ------------------------------------------------------------- async def refresh(self) -> None: @@ -1328,6 +1443,7 @@ def _create_sent_message( postable: AdapterPostableMessage, thread_id_override: str | None = None, raw: Any = None, + reply_to: Message | None = None, ) -> SentMessage: adapter = self.adapter thread_id = thread_id_override or self._id @@ -1338,7 +1454,10 @@ def _create_sent_message( async def _edit(new_content: Any) -> SentMessage: new_content = await thread_impl._process_callback_urls(new_content) await adapter.edit_message(thread_id, message_id, new_content) - return thread_impl._create_sent_message(message_id, new_content) + # Keep the resolved thread ID and reply target on the edited + # message (upstream ``createSentMessage(messageId, postable, + # threadId, replyTo)``). + return thread_impl._create_sent_message(message_id, new_content, thread_id, reply_to=reply_to) async def _delete() -> None: await adapter.delete_message(thread_id, message_id) @@ -1368,6 +1487,7 @@ async def _remove_reaction(emoji: EmojiValue | str) -> None: attachments=attachments, links=[], raw=raw, + reply_to=reply_to, _edit=_edit, _delete=_delete, _add_reaction=_add_reaction, @@ -1384,7 +1504,7 @@ def create_sent_message_from_message(self, message: Message) -> SentMessage: async def _edit(new_content: Any) -> SentMessage: new_content = await thread_impl._process_callback_urls(new_content) await adapter.edit_message(thread_id, message_id, new_content) - return thread_impl._create_sent_message(message_id, new_content, thread_id) + return thread_impl._create_sent_message(message_id, new_content, thread_id, reply_to=message.reply_to) async def _delete() -> None: await adapter.delete_message(thread_id, message_id) diff --git a/src/chat_sdk/types.py b/src/chat_sdk/types.py index 808c80f5..a250b439 100644 --- a/src/chat_sdk/types.py +++ b/src/chat_sdk/types.py @@ -1832,10 +1832,52 @@ async def post_ephemeral( thread_id: str, user_id: str, message: AdapterPostableMessage, - ) -> EphemeralMessage: - """Post an ephemeral message visible only to a specific user.""" + *, + options: PostEphemeralOptions | None = None, + ) -> EphemeralMessage | None: + """Post a message visible only to a specific user, natively or via an explicit fallback. + + ``options`` is the caller's :class:`PostEphemeralOptions`. Returns an + :class:`EphemeralMessage` (``used_fallback`` says whether delivery was + private), or ``None`` when the adapter has no private delivery path; + ``Thread.post_ephemeral`` returns that ``None`` unchanged. Platforms + with targeted messages (e.g. Teams) implement this natively. + + The SDK passes ``options=`` only to implementations whose signature + accepts it, so overrides written without the parameter keep working. + """ raise ChatNotImplementedError(self.name, "postEphemeral") + async def reply( + self, + thread_id: str, + message_id: str, + message: AdapterPostableMessage, + ) -> RawMessage: + """Post ``message`` as a native reply to ``message_id`` (quote, threaded reply). + + Optional: ``Thread.reply()`` raises + :class:`~chat_sdk.errors.ChatNotImplementedError` (``"replies"``) when + an adapter does not provide it. + """ + raise ChatNotImplementedError(self.name, "replies") + + async def mark_as_read( + self, + thread_id: str, + message_id: str, + message: Message | None = None, + ) -> None: + """Send a read receipt for an inbound message. + + Optional: ``Thread.mark_as_read()`` raises + :class:`~chat_sdk.errors.ChatNotImplementedError` (``"read-receipts"``) + when an adapter does not provide it. ``message`` is the full message + when the caller has one, so adapters can read platform data off + ``message.raw`` instead of resolving the ID themselves. + """ + raise ChatNotImplementedError(self.name, "read-receipts") + async def schedule_message( self, thread_id: str, @@ -2279,6 +2321,29 @@ async def get_participants(self) -> list[Author]: """Return unique non-bot, non-self authors who've posted in the thread.""" ... + async def reply( + self, + target: str | Message, + message: AdapterPostableMessage | AsyncIterable[Any], + ) -> SentMessage: + """Reply to a specific message with the platform's native reply. + + Raises :class:`~chat_sdk.errors.ChatNotImplementedError` on adapters + without native reply support. A ``Message`` target is checked against + this thread and carried to ``SentMessage.reply_to``; a raw ID is + resolved only against messages the thread already holds (never + fetched). Streams are buffered and posted as one message. + """ + ... + + async def mark_as_read(self, message: str | Message | None = None) -> None: + """Send a read receipt; defaults to the message being handled. + + Raises :class:`~chat_sdk.errors.ChatNotImplementedError` when the + adapter does not support read receipts. + """ + ... + def create_sent_message_from_message(self, message: Message) -> SentMessage: """Wrap a Message as a SentMessage with edit/delete capabilities.""" ... diff --git a/tests/test_channel_faithful.py b/tests/test_channel_faithful.py index b49ea855..2a59a850 100644 --- a/tests/test_channel_faithful.py +++ b/tests/test_channel_faithful.py @@ -928,7 +928,9 @@ async def test_should_use_adapter_postephemeral_when_available(self): channel = _make_channel(adapter, state) result = await channel.post_ephemeral("U456", "Secret!", PostEphemeralOptions(fallback_to_dm=True)) - mock_post_ephemeral.assert_called_once_with("slack:C123", "U456", "Secret!") + mock_post_ephemeral.assert_called_once_with( + "slack:C123", "U456", "Secret!", options=PostEphemeralOptions(fallback_to_dm=True) + ) assert result is not None assert result.id == "eph-1" assert result.thread_id == "slack:C123" @@ -965,7 +967,9 @@ async def test_should_extract_userid_from_author_object(self): await channel.post_ephemeral(author, "Hello!", PostEphemeralOptions(fallback_to_dm=False)) assert mock_post_ephemeral.call_count == 1 - mock_post_ephemeral.assert_called_once_with("slack:C123", "U789", "Hello!") + mock_post_ephemeral.assert_called_once_with( + "slack:C123", "U789", "Hello!", options=PostEphemeralOptions(fallback_to_dm=False) + ) # it("should return null when adapter has no postEphemeral and fallbackToDM is false") @pytest.mark.asyncio @@ -1015,6 +1019,26 @@ async def test_should_return_null_when_no_postephemeral_no_opendm_and_fallbackto assert result is None + # Python-specific: a custom adapter written against the older + # 3-argument ``post_ephemeral`` still works from a channel. + @pytest.mark.asyncio + async def test_three_argument_custom_post_ephemeral_still_works_from_a_channel(self): + adapter = create_mock_adapter() + calls: list[tuple[str, str, Any]] = [] + + async def legacy_post_ephemeral(thread_id: str, user_id: str, message: Any) -> EphemeralMessage: + calls.append((thread_id, user_id, message)) + return EphemeralMessage(id="eph-legacy", thread_id=thread_id, used_fallback=False, raw={}) + + adapter.post_ephemeral = legacy_post_ephemeral # type: ignore[attr-defined] + channel = _make_channel(adapter, create_mock_state()) + + result = await channel.post_ephemeral("U456", "Secret!", PostEphemeralOptions(fallback_to_dm=False)) + + assert calls == [("slack:C123", "U456", "Secret!")] + assert result is not None + assert result.id == "eph-legacy" + # =========================================================================== # ChannelImpl.startTyping diff --git a/tests/test_compat.py b/tests/test_compat.py new file mode 100644 index 00000000..71fe801e --- /dev/null +++ b/tests/test_compat.py @@ -0,0 +1,104 @@ +"""Tests for the Python-only adapter-signature probe (``chat_sdk._compat``).""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock + +import pytest + +from chat_sdk._compat import accepts_kwarg +from chat_sdk.errors import ChatNotImplementedError +from chat_sdk.testing import create_mock_state +from chat_sdk.thread import ThreadImpl, _ThreadImplConfig +from chat_sdk.types import BaseAdapter + + +async def _three_args(thread_id: str, user_id: str, message: Any) -> None: ... + + +async def _positional_options(thread_id: str, user_id: str, message: Any, options: Any = None) -> None: ... + + +async def _keyword_only_options(thread_id: str, user_id: str, message: Any, *, options: Any = None) -> None: ... + + +async def _var_kwargs(thread_id: str, user_id: str, message: Any, **kwargs: Any) -> None: ... + + +async def _positional_only_options(thread_id: str, user_id: str, message: Any, options: Any = None, /) -> None: ... + + +class _Adapter: + async def legacy(self, thread_id: str, user_id: str, message: Any) -> None: ... + + async def modern(self, thread_id: str, user_id: str, message: Any, *, options: Any = None) -> None: ... + + +class _Unhashable: + __hash__ = None # type: ignore[assignment] + + async def __call__(self, thread_id: str, user_id: str, message: Any, *, options: Any = None) -> None: ... + + +@pytest.mark.parametrize( + ("func", "expected"), + [ + (_three_args, False), + (_positional_options, True), + (_keyword_only_options, True), + (_var_kwargs, True), + (_positional_only_options, False), + (_Adapter().legacy, False), + (_Adapter().modern, True), + (AsyncMock(), True), + (_Unhashable(), True), + ], + ids=[ + "three-args", + "positional-or-keyword", + "keyword-only", + "var-kwargs", + "positional-only", + "bound-legacy", + "bound-modern", + "async-mock", + "unhashable-callable", + ], +) +def test_accepts_kwarg(func: Any, expected: bool) -> None: + assert accepts_kwarg(func, "options") is expected + + +class _MinimalBaseAdapter(BaseAdapter): + @property + def name(self) -> str: + return "custom" + + @property + def user_name(self) -> str: + return "custom-bot" + + +def _base_adapter_thread() -> ThreadImpl: + return ThreadImpl( + _ThreadImplConfig( + id="custom:C1:T1", + adapter=_MinimalBaseAdapter(), # type: ignore[arg-type] + state_adapter=create_mock_state(), + ) + ) + + +@pytest.mark.asyncio +async def test_base_adapter_default_reply_raises_replies_not_implemented() -> None: + with pytest.raises(ChatNotImplementedError) as exc_info: + await _base_adapter_thread().reply("m1", "Hello") + assert (exc_info.value.adapter, exc_info.value.method) == ("custom", "replies") + + +@pytest.mark.asyncio +async def test_base_adapter_default_mark_as_read_raises_read_receipts_not_implemented() -> None: + with pytest.raises(ChatNotImplementedError) as exc_info: + await _base_adapter_thread().mark_as_read("m1") + assert (exc_info.value.adapter, exc_info.value.method) == ("custom", "read-receipts") diff --git a/tests/test_thread_faithful.py b/tests/test_thread_faithful.py index 24e8f6c0..59f67dce 100644 --- a/tests/test_thread_faithful.py +++ b/tests/test_thread_faithful.py @@ -21,7 +21,7 @@ from chat_sdk.callback_url import decode_callback_value from chat_sdk.cards import Actions, Button, Card from chat_sdk.channel import derive_channel_id -from chat_sdk.errors import ChatNotImplementedError +from chat_sdk.errors import ChatError, ChatNotImplementedError from chat_sdk.plan import StreamingPlan, StreamingPlanOptions from chat_sdk.shared.mock_adapter import MockLogger from chat_sdk.testing import ( @@ -1931,7 +1931,9 @@ async def test_should_use_adapter_postephemeral_when_available(self): thread = _make_thread(adapter, state) result = await thread.post_ephemeral("U456", "Secret message", PostEphemeralOptions(fallback_to_dm=True)) - mock_post_ephemeral.assert_called_once_with("slack:C123:1234.5678", "U456", "Secret message") + mock_post_ephemeral.assert_called_once_with( + "slack:C123:1234.5678", "U456", "Secret message", options=PostEphemeralOptions(fallback_to_dm=True) + ) assert result is not None assert result.id == "ephemeral-1" assert result.thread_id == "slack:C123:1234.5678" @@ -1968,7 +1970,9 @@ async def test_should_extract_userid_from_author_object(self): await thread.post_ephemeral(author, "Secret message", PostEphemeralOptions(fallback_to_dm=True)) assert mock_post_ephemeral.call_count == 1 - mock_post_ephemeral.assert_called_once_with("slack:C123:1234.5678", "U789", "Secret message") + mock_post_ephemeral.assert_called_once_with( + "slack:C123:1234.5678", "U789", "Secret message", options=PostEphemeralOptions(fallback_to_dm=True) + ) # it("should fallback to DM when adapter has no postEphemeral and fallbackToDM is true") @pytest.mark.asyncio @@ -2039,6 +2043,43 @@ async def test_should_return_null_when_adapter_has_no_postephemeral_or_opendm(se # Should return None since no fallback is possible assert result is None + # Python-specific: a custom adapter written against the older + # 3-argument ``post_ephemeral`` still works (``options=`` is only passed + # to implementations that accept it). + @pytest.mark.asyncio + async def test_three_argument_custom_post_ephemeral_still_works(self): + adapter = create_mock_adapter() + calls: list[tuple[str, str, Any]] = [] + + async def legacy_post_ephemeral(thread_id: str, user_id: str, message: Any) -> EphemeralMessage: + calls.append((thread_id, user_id, message)) + return EphemeralMessage(id="eph-legacy", thread_id=thread_id, used_fallback=False, raw={}) + + adapter.post_ephemeral = legacy_post_ephemeral # type: ignore[attr-defined] + thread = _make_thread(adapter) + + result = await thread.post_ephemeral("U456", "Secret message", PostEphemeralOptions(fallback_to_dm=True)) + + assert calls == [("slack:C123:1234.5678", "U456", "Secret message")] + assert result is not None + assert result.id == "eph-legacy" + + # Python-specific: an adapter that returns ``None`` (no private delivery + # path) yields ``None`` with no DM fallback, as upstream does. + @pytest.mark.asyncio + async def test_adapter_returning_none_yields_none_without_dm_fallback(self): + adapter = create_mock_adapter() + adapter.post_ephemeral = AsyncMock(return_value=None) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + result = await thread.post_ephemeral("U456", "Secret message", PostEphemeralOptions(fallback_to_dm=True)) + + assert result is None + adapter.post_ephemeral.assert_awaited_once_with( # type: ignore[attr-defined] + "slack:C123:1234.5678", "U456", "Secret message", options=PostEphemeralOptions(fallback_to_dm=True) + ) + assert adapter._post_calls == [] + # =========================================================================== # post with Plan @@ -2815,6 +2856,112 @@ async def test_should_pass_status_to_adapterstarttyping(self): assert adapter._start_typing_calls[0] == ("slack:C123:1234.5678", "thinking...") +# =========================================================================== +# markAsRead +# =========================================================================== + + +class TestMarkAsRead: + """describe("markAsRead")""" + + # it("marks the current message when no target is provided") + @pytest.mark.asyncio + async def test_marks_the_current_message_when_no_target_is_provided(self): + adapter = create_mock_adapter() + adapter.mark_as_read = AsyncMock(return_value=None) + message = create_test_message("msg-1", "Hello") + thread = _make_thread(adapter, current_message=message) + + await thread.mark_as_read() + + adapter.mark_as_read.assert_awaited_once_with("slack:C123:1234.5678", "msg-1", message) + + # it("marks an explicit message id") + @pytest.mark.asyncio + async def test_marks_an_explicit_message_id(self): + adapter = create_mock_adapter() + adapter.mark_as_read = AsyncMock(return_value=None) + thread = _make_thread(adapter) + + await thread.mark_as_read("msg-2") + + adapter.mark_as_read.assert_awaited_once_with("slack:C123:1234.5678", "msg-2", None) + + # it("marks an explicit message") + @pytest.mark.asyncio + async def test_marks_an_explicit_message(self): + adapter = create_mock_adapter() + adapter.mark_as_read = AsyncMock(return_value=None) + message = create_test_message("msg-3", "Hello") + thread = _make_thread(adapter) + + await thread.mark_as_read(message) + + adapter.mark_as_read.assert_awaited_once_with("slack:C123:1234.5678", "msg-3", message) + + # it("rejects a message from another thread") + @pytest.mark.asyncio + async def test_rejects_a_message_from_another_thread(self): + adapter = create_mock_adapter() + adapter.mark_as_read = AsyncMock(return_value=None) + message = create_test_message("msg-4", "Hello", thread_id="slack:C999:9999.0000") + thread = _make_thread(adapter) + + with pytest.raises(ChatError, match="Cannot mark a message from another thread as read"): + await thread.mark_as_read(message) + adapter.mark_as_read.assert_not_awaited() + + # it("requires a target outside a message handler") + @pytest.mark.asyncio + async def test_requires_a_target_outside_a_message_handler(self): + adapter = create_mock_adapter() + adapter.mark_as_read = AsyncMock(return_value=None) + thread = _make_thread(adapter) + + with pytest.raises(ChatError, match="A message is required outside a message handler"): + await thread.mark_as_read() + adapter.mark_as_read.assert_not_awaited() + + # it("throws when the adapter does not support read receipts") + @pytest.mark.asyncio + async def test_throws_when_the_adapter_does_not_support_read_receipts(self): + adapter = create_mock_adapter() + adapter.mark_as_read = None + thread = _make_thread(adapter, current_message=create_test_message("msg-5", "Hello")) + + with pytest.raises(ChatNotImplementedError) as exc_info: + await thread.mark_as_read() + assert exc_info.value.method == "read-receipts" + + # it("preserves the current message through serialization") + @pytest.mark.asyncio + async def test_preserves_the_current_message_through_serialization(self): + adapter = create_mock_adapter() + adapter.mark_as_read = AsyncMock(return_value=None) + original = _make_thread(adapter, current_message=create_test_message("msg-6", "Hello")) + restored = ThreadImpl.from_json(original.to_json(), adapter=adapter) + + await restored.mark_as_read() + + adapter.mark_as_read.assert_awaited_once() + thread_id, message_id, message = adapter.mark_as_read.await_args.args + assert (thread_id, message_id) == ("slack:C123:1234.5678", "msg-6") + assert isinstance(message, Message) + assert message.id == "msg-6" + + # Python-specific: ``None`` means "current message", but an explicit + # empty ID is not replaced by it (upstream ``message ?? current`` keeps + # ``""``, then ``!target`` rejects it). + @pytest.mark.asyncio + async def test_empty_message_id_is_rejected_even_with_a_current_message(self): + adapter = create_mock_adapter() + thread = _make_thread(adapter, current_message=create_test_message("msg-7", "Hello")) + + with pytest.raises(ChatError, match="A message is required outside a message handler"): + await thread.mark_as_read("") + adapter.mark_as_read.assert_not_awaited() + + # =========================================================================== # mentionUser # =========================================================================== @@ -3116,6 +3263,252 @@ async def test_should_serialize_a_sent_message_via_tojson(self): assert result.author.is_me is True +# =========================================================================== +# reply() +# =========================================================================== + + +def _reply_raw() -> RawMessage: + return RawMessage(id="reply-1", thread_id="slack:C123:1234.5678", raw={}) + + +class TestReply: + """describe("reply()")""" + + # it("throws when the adapter does not support replies") + @pytest.mark.asyncio + async def test_throws_when_the_adapter_does_not_support_replies(self): + thread = _make_thread() + + with pytest.raises(ChatNotImplementedError) as exc_info: + await thread.reply("original", "Hello") + assert exc_info.value.method == "replies" + + # it("delegates a message id and content to the adapter") + @pytest.mark.asyncio + async def test_delegates_a_message_id_and_content_to_the_adapter(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + result = await thread.reply("original", PostableMarkdown(markdown="Hello")) + + adapter.reply.assert_awaited_once_with( # type: ignore[attr-defined] + "slack:C123:1234.5678", "original", PostableMarkdown(markdown="Hello") + ) + assert result.id == "reply-1" + + # it("accepts a Message and preserves it on the result") + @pytest.mark.asyncio + async def test_accepts_a_message_and_preserves_it_on_the_result(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + original = create_test_message("original", "Question") + + result = await thread.reply(original, "Answer") + + adapter.reply.assert_awaited_once_with("slack:C123:1234.5678", "original", "Answer") # type: ignore[attr-defined] + assert result.reply_to is original + + # it("rejects a Message from another thread") + @pytest.mark.asyncio + async def test_rejects_a_message_from_another_thread(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + original = create_test_message("original", "Question", thread_id="slack:C999:9999.0000") + + with pytest.raises(ChatError, match="Cannot reply to a message from another thread"): + await thread.reply(original, "Answer") + adapter.reply.assert_not_awaited() # type: ignore[attr-defined] + + # it("converts JSX cards before delegating") + # Python has no JSX; a CardElement dict is passed through to the adapter. + @pytest.mark.asyncio + async def test_converts_jsx_cards_before_delegating(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + await thread.reply("original", Card(title="Answer")) + + thread_id, target_id, postable = adapter.reply.await_args.args # type: ignore[attr-defined] + assert (thread_id, target_id) == ("slack:C123:1234.5678", "original") + assert postable["type"] == "card" + assert postable["title"] == "Answer" + + # it("buffers streams into one markdown reply") + @pytest.mark.asyncio + async def test_buffers_streams_into_one_markdown_reply(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + await thread.reply("original", _create_text_stream(["Hello ", "world"])) + + adapter.reply.assert_awaited_once_with( # type: ignore[attr-defined] + "slack:C123:1234.5678", "original", PostableMarkdown(markdown="Hello world") + ) + + # it("falls back to a space when a stream produces no text") + @pytest.mark.asyncio + async def test_falls_back_to_a_space_when_a_stream_produces_no_text(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + async def stream() -> AsyncIterator[Any]: + yield {"type": "tool-call", "toolName": "search"} + yield {"type": "finish-step"} + + await thread.reply("original", stream()) + + adapter.reply.assert_awaited_once_with( # type: ignore[attr-defined] + "slack:C123:1234.5678", "original", PostableMarkdown(markdown=" ") + ) + + # it("resolves a message id against messages the thread knows") + @pytest.mark.asyncio + async def test_resolves_a_message_id_against_messages_the_thread_knows(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + original = create_test_message("original", "Question") + thread.recent_messages = [original] + + result = await thread.reply("original", "Answer") + + assert result.reply_to is original + + # it("leaves replyTo undefined for an unknown message id") + @pytest.mark.asyncio + async def test_leaves_replyto_undefined_for_an_unknown_message_id(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + adapter.fetch_message = AsyncMock(return_value=None) # type: ignore[method-assign] + thread = _make_thread(adapter) + + result = await thread.reply("not-in-memory", "Answer") + + adapter.reply.assert_awaited_once_with("slack:C123:1234.5678", "not-in-memory", "Answer") # type: ignore[attr-defined] + assert result.reply_to is None + # The ID is never resolved by fetching. + adapter.fetch_message.assert_not_awaited() + + # Python-specific: an ID matching the message being handled resolves to + # it (``_find_known_message`` checks the current message first). + @pytest.mark.asyncio + async def test_resolves_the_current_message_id_to_the_current_message(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + current = create_test_message("current", "Question") + thread = _make_thread(adapter, current_message=current) + + result = await thread.reply("current", "Answer") + + assert result.reply_to is current + + # Python-specific: streams keep text and ``markdown_text`` chunks and drop + # structured ``task_update`` chunks. + @pytest.mark.asyncio + async def test_buffers_markdown_text_chunks_and_drops_task_updates(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + async def stream() -> AsyncIterator[Any]: + yield "Hi " + yield TaskUpdateChunk(id="t1", title="Searching", status="in_progress") + yield MarkdownTextChunk(text="**there**") + yield {"type": "markdown_text", "text": "!"} + + await thread.reply("original", stream()) + + adapter.reply.assert_awaited_once_with( # type: ignore[attr-defined] + "slack:C123:1234.5678", "original", PostableMarkdown(markdown="Hi **there**!") + ) + + # Python-specific: the reply is cached in thread history with its target. + @pytest.mark.asyncio + async def test_appends_the_reply_to_thread_history(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + history = AsyncMock() + thread = ThreadImpl( + _ThreadImplConfig( + id="slack:C123:1234.5678", + adapter=adapter, + state_adapter=create_mock_state(), + channel_id="C123", + thread_history=history, + ) + ) + original = create_test_message("original", "Question") + + await thread.reply(original, "Answer") + + history.append.assert_awaited_once() + thread_id, cached = history.append.await_args.args + assert thread_id == "slack:C123:1234.5678" + assert cached.id == "reply-1" + assert cached.text == "Answer" + assert cached.reply_to is original + + # Python-specific: ``SentMessage.edit()`` keeps the reply target. + @pytest.mark.asyncio + async def test_edit_after_reply_keeps_reply_to(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + original = create_test_message("original", "Question") + + sent = await thread.reply(original, "Answer") + edited = await sent.edit("Better answer") + + assert edited.reply_to is original + assert edited.text == "Better answer" + + # Python-specific: callback URLs in a reply card are minted with this + # thread's scope, as for ``post()``. + @pytest.mark.asyncio + async def test_reply_encodes_callback_urls_with_thread_scope(self): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + state = create_mock_state() + thread = _make_thread(adapter, state) + + await thread.reply("original", _make_card_with_callback("https://example.com/cb")) + + sent_card = adapter.reply.await_args.args[2] # type: ignore[attr-defined] + button = sent_card["children"][0]["children"][0] + token = decode_callback_value(button["value"]).callback_token + assert token is not None + stored = await state.get(f"chat:callback:{token}") + assert stored is not None + assert stored["url"] == "https://example.com/cb" + assert stored["scope"] == {"id": "slack:C123:1234.5678", "type": "thread"} + + +class TestSentMessageEditThreadId: + """Python-specific: edits keep the thread ID the adapter returned.""" + + @pytest.mark.asyncio + async def test_edit_keeps_thread_id_override(self): + adapter = create_mock_adapter() + adapter.post_message = AsyncMock( # type: ignore[method-assign] + return_value=RawMessage(id="msg-9", thread_id="slack:C123:9999.0000", raw={}) + ) + thread = _make_thread(adapter) + + sent = await thread.post("Hello") + edited = await sent.edit("Updated") + + assert sent.thread_id == "slack:C123:9999.0000" + assert edited.thread_id == "slack:C123:9999.0000" + assert adapter._edit_calls == [("slack:C123:9999.0000", "msg-9", "Updated")] + + # =========================================================================== # schedule() # =========================================================================== From 4e078bb03ed71b2df37aa148861e9c73d2267dd8 Mon Sep 17 00:00:00 2001 From: patrick-chinchill Date: Thu, 1 Oct 2026 00:11:52 -0700 Subject: [PATCH 2/2] address review: no raising BaseAdapter reply/mark_as_read defaults, JS trim for empty-stream check, edit reply_to test - Remove the concrete BaseAdapter.reply / mark_as_read bodies (documented as optional hooks instead) so Thread.reply()/mark_as_read() raise ChatNotImplementedError before consuming a stream, minting callback tokens or raising MESSAGE_REQUIRED, matching upstream thread.ts fail-first order. - Empty-stream fallback in Thread.reply strips JS_WHITESPACE (JS trim()). - Tests: BaseAdapter fail-first regressions, whitespace-only stream, create_sent_message_from_message edit keeps reply_to. --- CHANGELOG.md | 2 +- docs/UPSTREAM_SYNC.md | 18 +++++++----- scripts/fidelity_target.json | 4 +-- src/chat_sdk/thread.py | 6 ++-- src/chat_sdk/types.py | 53 +++++++++++++++-------------------- tests/test_compat.py | 39 ++++++++++++++++++++++---- tests/test_thread_faithful.py | 35 +++++++++++++++++++++++ 7 files changed, 109 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d166d06..0cc77624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ Sync wave from `chat@4.31.0` to `chat@4.41.1` (tracking #184). `UPSTREAM_PARITY` - **Core: `Thread.reply`, `Thread.mark_as_read`, `post_ephemeral` options** (#200). Ports vercel/chat `83ede7ea` (#819) and `18d4a230` (#820) from chat@4.38.0, the doc-only core part of `160140e3` (#737, chat@4.35.0) and the core slice of `bfee00af` (#939, chat@4.41.0). Core API only; adapter support comes in #228 (Telegram), #239 (WhatsApp/Messenger), #219 (Teams) and #241 (Gmail). - New `await thread.reply(target, message)` posts a native reply to a `Message` or message ID. A `Message` from another thread raises `ChatError`. A string ID is resolved only against the current message and `recent_messages`, and is never fetched. Streams are buffered into one markdown reply (`" "` if they produce no text). The returned `SentMessage.reply_to` is the target, and the reply is appended to thread history. - New `await thread.mark_as_read(message=None)` sends a read receipt. It defaults to the message being handled. With no message available it raises `ChatError("A message is required outside a message handler")`, and for a `Message` from another thread it raises `ChatError("Cannot mark a message from another thread as read")`. - - Optional adapter hooks `reply(thread_id, message_id, message) -> RawMessage` and `mark_as_read(thread_id, message_id, message=None)` are declared on `BaseAdapter` (not on the `Adapter` Protocol). Both are also on the `Thread` Protocol. + - Optional adapter hooks `reply(thread_id, message_id, message) -> RawMessage` and `mark_as_read(thread_id, message_id, message=None)` are documented on `BaseAdapter` without a default implementation (and are not on the `Adapter` Protocol), so an adapter without them fails with `ChatNotImplementedError` before a stream is consumed or a callback token is minted, as upstream. Both are also on the `Thread` Protocol. - **Consumer-visible:** `thread.reply()` / `thread.mark_as_read()` raise `ChatNotImplementedError` (`"replies"` / `"read-receipts"`) on adapters without the hook. No in-repo adapter has `reply` until #228/#239. Until #239, `thread.mark_as_read()` on WhatsApp raises `TypeError`, because `WhatsAppAdapter.mark_as_read(message_id)` keeps its one-argument signature. - **Consumer-visible:** `Thread.post_ephemeral` / `Channel.post_ephemeral` now pass the caller's `PostEphemeralOptions` to the adapter as `options=`. An adapter may return `None` (no private delivery path), which is returned unchanged with no DM fallback, as upstream. `BaseAdapter.post_ephemeral` is now `(thread_id, user_id, message, *, options=None) -> EphemeralMessage | None`. Slack and Google Chat accept `options` and ignore it. Tests that assert the adapter call with three arguments need `options=...` added. - **Fix:** `SentMessage.edit()` keeps the thread ID the adapter returned for the original post (it used to fall back to the thread's own ID) and keeps `reply_to`, as upstream. diff --git a/docs/UPSTREAM_SYNC.md b/docs/UPSTREAM_SYNC.md index 5c14d342..465e5b37 100644 --- a/docs/UPSTREAM_SYNC.md +++ b/docs/UPSTREAM_SYNC.md @@ -1969,13 +1969,17 @@ targeted ephemeral #219, Gmail #241. is rejected as `MESSAGE_REQUIRED`, as upstream (`"" ?? current` keeps `""`, then `!target` throws). The adapter receives `(thread_id, message_id, message_or_None)`. -- **Optional hooks.** `reply` and `mark_as_read` are declared on - `BaseAdapter` with defaults that raise `ChatNotImplementedError` with - `"replies"` / `"read-receipts"`, the same features `Thread` raises. They - are not on the `Adapter` Protocol, where they would become required. - `Thread` finds the hooks with `getattr(adapter, name, None)`. A - `BaseAdapter` subclass without an override therefore reaches the default, - which raises the same error after buffering and callback processing. +- **Optional hooks.** `reply` and `mark_as_read` are documented on + `BaseAdapter` (a comment block with their signatures) but have no default + method there, and they are not on the `Adapter` Protocol, where they would + become required. `Thread` finds the hooks with `getattr(adapter, name, + None)`, so an adapter without them, `BaseAdapter` subclasses included, + raises `ChatNotImplementedError` (`"replies"` / `"read-receipts"`) before + any other work, as upstream's `if (!this.adapter.reply)`: no stream is + consumed, no callback token is minted, and `mark_as_read()` outside a + handler reports the missing hook rather than `MESSAGE_REQUIRED`. A raising + default would have made every subclass look capable. The empty-stream + check strips with `JS_WHITESPACE` (JS `trim()`), not `str.strip()`. The shared mock adapter has a recording `mark_as_read` `AsyncMock` by default, as upstream's mock does, and no `reply`. - **Errors.** Python's `ChatError` has no `code`, so `MESSAGE_REQUIRED` and diff --git a/scripts/fidelity_target.json b/scripts/fidelity_target.json index a6b8952b..e42fd0ae 100644 --- a/scripts/fidelity_target.json +++ b/scripts/fidelity_target.json @@ -8,7 +8,7 @@ "matched_exact": 885, "matched_fuzzy": 75, "missing": 104, - "extra": 526 + "extra": 528 }, "absent_ts_files": [], "files": { @@ -48,7 +48,7 @@ "matched_exact": 153, "matched_fuzzy": 5, "missing_count": 1, - "extra_count": 41, + "extra_count": 43, "missing": [ ["startTyping", "passes the initiating user and clears processing after posting"] ], diff --git a/src/chat_sdk/thread.py b/src/chat_sdk/thread.py index 374a3d70..94b064a4 100644 --- a/src/chat_sdk/thread.py +++ b/src/chat_sdk/thread.py @@ -20,6 +20,7 @@ from chat_sdk.from_full_stream import from_full_stream from chat_sdk.logger import Logger from chat_sdk.plan import is_postable_object, post_postable_object +from chat_sdk.shared._js_compat import JS_WHITESPACE from chat_sdk.shared.streaming_markdown import StreamingMarkdownRenderer from chat_sdk.types import ( THREAD_STATE_TTL_MS, @@ -779,8 +780,9 @@ async def reply( markdown += chunk.get("text", "") # A stream can finish without producing text (tool calls only, or # just task_update/plan_update chunks). Platforms reject empty - # bodies, so fall back to a single space as upstream does. - postable = PostableMarkdown(markdown=markdown if markdown.strip() else " ") + # bodies, so fall back to a single space as upstream does. JS + # ``trim()`` whitespace set, not ``str.strip()``'s. + postable = PostableMarkdown(markdown=markdown if markdown.strip(JS_WHITESPACE) else " ") else: postable = message # type: ignore[assignment] diff --git a/src/chat_sdk/types.py b/src/chat_sdk/types.py index a250b439..d77a39c4 100644 --- a/src/chat_sdk/types.py +++ b/src/chat_sdk/types.py @@ -1771,7 +1771,9 @@ class BaseAdapter: Concrete adapters should inherit from this class and override methods they support. Required methods (from :class:`Adapter`) must still be implemented by the subclass. Optional methods raise - :class:`~chat_sdk.errors.ChatNotImplementedError` by default. + :class:`~chat_sdk.errors.ChatNotImplementedError` by default, except + the optional ``reply`` and ``mark_as_read`` hooks, which have no default + (see the comment above ``schedule_message``). """ # -- Required properties (must be overridden) ---------------------------- @@ -1848,35 +1850,26 @@ async def post_ephemeral( """ raise ChatNotImplementedError(self.name, "postEphemeral") - async def reply( - self, - thread_id: str, - message_id: str, - message: AdapterPostableMessage, - ) -> RawMessage: - """Post ``message`` as a native reply to ``message_id`` (quote, threaded reply). - - Optional: ``Thread.reply()`` raises - :class:`~chat_sdk.errors.ChatNotImplementedError` (``"replies"``) when - an adapter does not provide it. - """ - raise ChatNotImplementedError(self.name, "replies") - - async def mark_as_read( - self, - thread_id: str, - message_id: str, - message: Message | None = None, - ) -> None: - """Send a read receipt for an inbound message. - - Optional: ``Thread.mark_as_read()`` raises - :class:`~chat_sdk.errors.ChatNotImplementedError` (``"read-receipts"``) - when an adapter does not provide it. ``message`` is the full message - when the caller has one, so adapters can read platform data off - ``message.raw`` instead of resolving the ID themselves. - """ - raise ChatNotImplementedError(self.name, "read-receipts") + # -- Optional hooks without a default ----------------------------------- + # + # ``reply`` and ``mark_as_read`` are deliberately NOT defined here. + # ``Thread.reply()`` / ``Thread.mark_as_read()`` look them up with + # ``getattr(adapter, name, None)`` and raise ``ChatNotImplementedError`` + # (``"replies"`` / ``"read-receipts"``) *before* any other work when they + # are absent, as upstream's ``if (!this.adapter.reply)`` does. A raising + # default here would make every subclass look capable, so ``reply()`` + # would drain the caller's stream and mint callback tokens before + # failing. Subclasses that support them define: + # + # async def reply(self, thread_id: str, message_id: str, + # message: AdapterPostableMessage) -> RawMessage + # Post ``message`` as a native reply to ``message_id``. + # + # async def mark_as_read(self, thread_id: str, message_id: str, + # message: Message | None = None) -> None + # Send a read receipt. ``message`` is the full message when the + # caller has one, so adapters can read platform data off + # ``message.raw`` instead of resolving the ID themselves. async def schedule_message( self, diff --git a/tests/test_compat.py b/tests/test_compat.py index 71fe801e..7bad844e 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -2,12 +2,14 @@ from __future__ import annotations +from collections.abc import AsyncIterator from typing import Any from unittest.mock import AsyncMock import pytest from chat_sdk._compat import accepts_kwarg +from chat_sdk.cards import Actions, Button, Card from chat_sdk.errors import ChatNotImplementedError from chat_sdk.testing import create_mock_state from chat_sdk.thread import ThreadImpl, _ThreadImplConfig @@ -80,25 +82,50 @@ def user_name(self) -> str: return "custom-bot" -def _base_adapter_thread() -> ThreadImpl: +def _base_adapter_thread(state: Any = None) -> ThreadImpl: return ThreadImpl( _ThreadImplConfig( id="custom:C1:T1", adapter=_MinimalBaseAdapter(), # type: ignore[arg-type] - state_adapter=create_mock_state(), + state_adapter=state if state is not None else create_mock_state(), ) ) +# A BaseAdapter subclass that does not define ``reply`` / ``mark_as_read`` is +# unsupported, and the error comes first, as upstream's ``if +# (!this.adapter.reply)`` check: no stream is consumed and no callback token is +# minted before ``ChatNotImplementedError``. + + @pytest.mark.asyncio -async def test_base_adapter_default_reply_raises_replies_not_implemented() -> None: +async def test_base_adapter_without_reply_raises_before_consuming_the_stream() -> None: + consumed: list[str] = [] + + async def stream() -> AsyncIterator[str]: + for chunk in ("a", "b"): + consumed.append(chunk) + yield chunk + with pytest.raises(ChatNotImplementedError) as exc_info: - await _base_adapter_thread().reply("m1", "Hello") + await _base_adapter_thread().reply("m1", stream()) assert (exc_info.value.adapter, exc_info.value.method) == ("custom", "replies") + assert consumed == [] + + +@pytest.mark.asyncio +async def test_base_adapter_without_reply_raises_before_minting_callback_tokens() -> None: + state = create_mock_state() + card = Card(children=[Actions([Button(id="b", label="Go", callback_url="https://example.com/cb")])]) + + with pytest.raises(ChatNotImplementedError): + await _base_adapter_thread(state).reply("m1", card) + assert [key for key in state.cache if key.startswith("chat:callback:")] == [] @pytest.mark.asyncio -async def test_base_adapter_default_mark_as_read_raises_read_receipts_not_implemented() -> None: +async def test_base_adapter_without_mark_as_read_raises_not_implemented_outside_a_handler() -> None: + # No current message: upstream reports the missing hook, not MESSAGE_REQUIRED. with pytest.raises(ChatNotImplementedError) as exc_info: - await _base_adapter_thread().mark_as_read("m1") + await _base_adapter_thread().mark_as_read() assert (exc_info.value.adapter, exc_info.value.method) == ("custom", "read-receipts") diff --git a/tests/test_thread_faithful.py b/tests/test_thread_faithful.py index 59f67dce..b76ac5a7 100644 --- a/tests/test_thread_faithful.py +++ b/tests/test_thread_faithful.py @@ -3090,6 +3090,21 @@ def test_should_preserve_ismention_from_original_message(self): sent = thread.create_sent_message_from_message(msg) assert sent.is_mention is True + # Python-specific: ``edit()`` on the wrapped message keeps its + # ``reply_to`` (upstream ``createSentMessage(..., threadId, replyTo)``). + @pytest.mark.asyncio + async def test_edit_of_a_wrapped_message_keeps_reply_to(self): + adapter = create_mock_adapter() + thread = _make_thread(adapter) + original = create_test_message("msg-0", "Question") + msg = create_test_message("msg-1", "Answer", reply_to=original) + + edited = await thread.create_sent_message_from_message(msg).edit("Better answer") + + assert edited.reply_to is original + assert edited.text == "Better answer" + assert adapter._edit_calls == [("slack:C123:1234.5678", "msg-1", "Better answer")] + # it("should provide toJSON that delegates to the original message") # Note: Python SentMessage doesn't have to_json; skipping or adapting # We test via the underlying Message.to_json behavior @@ -3368,6 +3383,26 @@ async def stream() -> AsyncIterator[Any]: "slack:C123:1234.5678", "original", PostableMarkdown(markdown=" ") ) + # Python-specific: the empty-stream check uses JS ``trim()``'s whitespace + # set. Whitespace-only text (and U+FEFF, which JS trims) posts " "; + # U+001C, which ``str.strip()`` would drop but JS keeps, is sent as is. + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("chunks", "expected"), + [([" ", "\n"], " "), (["\ufeff"], " "), (["\x1c"], "\x1c")], + ids=["spaces-and-newline", "bom", "file-separator"], + ) + async def test_whitespace_only_stream_uses_js_trim(self, chunks: list[str], expected: str): + adapter = create_mock_adapter() + adapter.reply = AsyncMock(return_value=_reply_raw()) # type: ignore[attr-defined] + thread = _make_thread(adapter) + + await thread.reply("original", _create_text_stream(chunks)) + + adapter.reply.assert_awaited_once_with( # type: ignore[attr-defined] + "slack:C123:1234.5678", "original", PostableMarkdown(markdown=expected) + ) + # it("resolves a message id against messages the thread knows") @pytest.mark.asyncio async def test_resolves_a_message_id_against_messages_the_thread_knows(self):