fix(slack): route empty-DM thread_ts fetch to conversations.history (#138) (#175)

A top-level Slack DM root encodes an empty thread_ts (slack:Dxxx:). The
fetch paths previously called conversations.replies(ts="") unconditionally,
which returns no replies and loses the DM root context for every DM.

- fetch_messages: when thread_ts is empty (falsy), route to the existing
  channel-history helpers (_fetch_channel_messages_forward /
  _fetch_channel_messages_backward, both conversations.history) instead of
  the thread-reply helpers. Direction, limit, and cursor semantics are
  preserved; for a DM the channel IS the conversation, so this is correct.
- fetch_message: when thread_ts is empty, fetch the single message via
  conversations.history(latest=message_id, inclusive=True, limit=1),
  mirroring the inner link-preview fetch_message.
- Non-empty thread_ts stays byte-identical (still conversations.replies).

This is a divergence ahead of upstream — upstream's fetchMessages /
fetchMessage call conversations.replies(ts: threadTs) with no empty-thread_ts
guard either. Documented in docs/UPSTREAM_SYNC.md as a candidate to file
upstream; it also covers the #137 DM block-action consumer uniformly.

Tests: empty-DM fetch_messages (forward + backward) and fetch_message assert
conversations.history is used and conversations.replies(ts="") is never
called; non-empty thread_ts regression guards assert conversations.replies is
still used (so the routing cannot over-trigger).

Author: patrick-chinchill

docs/UPSTREAM_SYNC.md

@@ -667,7 +667,8 @@ stay explicit instead of being rediscovered in code review.
 | `_rehydrate_message` with `Message` input | Falls through to the `rehydrate_attachment` pass even when the dequeued entry is already a `Message` instance | Early-returns on `raw instanceof Message` before rehydration | The Python port's Redis + Postgres `dequeue()` upgrade raw JSON to `Message.from_json(...)` before returning (upstream's dequeue returns the raw JSON.parse'd dict). Upstream's `instanceof Message` shortcut therefore only fires for in-memory state, but ours would fire for persistent backends too, leaving `fetch_data` stripped forever. The rehydrate pass still skips any attachment that already has `fetch_data`, so in-memory callers pay no cost. |
 | Slack Socket Mode reconnect loop | Outer reconnect loop on top of `slack_sdk.socket_mode.aiohttp.SocketModeClient` (which itself has `auto_reconnect_enabled=True`). Exponential backoff (1s → 30s) with explicit shutdown signaling and a tracked `asyncio.Task` so `disconnect()` can cancel cleanly | Single `SocketModeClient` instance from `@slack/socket-mode`; relies entirely on the package's internal reconnect | Hazard #5 (async task lifecycle): a long-lived WebSocket needs an explicit shutdown path so `disconnect()` doesn't leak the loop, and a guarded outer reconnect path so the adapter survives `connect()` itself raising (which the inner client doesn't retry). Inner auto-reconnect still runs; the outer loop is belt-and-suspenders, not a divergence in observable behavior. |
 | Slack Socket Mode listener serverless variant | Not ported | `startSocketModeListener()` / `runSocketModeListener()` open a transient socket for `durationMs` and forward events via HTTP POST | Vercel-specific pattern (cron-triggered ephemeral listener with `waitUntil`). The forwarded-event receiver (`x-slack-socket-token` handling in `handle_webhook`) is ported so a separate Python process can run the long-lived listener; the deployment glue itself isn't part of the SDK. |
-| Slack DM block-action threading (#133/#137) | `_handle_block_actions` sets `thread_ts=""` for a top-level DM button click (never falls back to the clicked message's own `ts`), so a handler's `event.thread.post(...)` does not spawn a phantom "1 reply" thread in the DM. Mirrors `_handle_message_event`'s DM handling (`thread_ts=""` for top-level DMs). | `handleBlockActions` (`adapter-slack/src/index.ts:1455-1456,1470`) computes `thread_ts \|\| container.thread_ts \|\| messageTs` and encodes `threadTs \|\| messageTs \|\| ""` — it falls back to the clicked message's `ts` even for DMs, so a DM button click spawns a phantom reply thread. Upstream's `handleMessageEvent` *does* empty-case DMs (`:2158`), but `handleBlockActions` does **not** — an upstream internal inconsistency. | Hard UX failure with no workaround (phantom "1 reply" threads on DM button clicks). We extend upstream's own DM-message convention to the block-action path. The resulting empty DM `thread_ts` is consumed unguarded by `fetch_messages` → `conversations.replies(ts="")` — identical to upstream (`fetchMessages` `:4178` has no empty-`thread_ts` guard) and to the faithful DM-message path; a `conversations.history` fallback for empty DM `thread_ts` is a separate, codebase-wide follow-up. The block-action fix should be contributed upstream (cf. PR #107's stream() divergence) to restore parity. |
+| Slack DM block-action threading (#133/#137) | `_handle_block_actions` sets `thread_ts=""` for a top-level DM button click (never falls back to the clicked message's own `ts`), so a handler's `event.thread.post(...)` does not spawn a phantom "1 reply" thread in the DM. Mirrors `_handle_message_event`'s DM handling (`thread_ts=""` for top-level DMs). | `handleBlockActions` (`adapter-slack/src/index.ts:1455-1456,1470`) computes `thread_ts \|\| container.thread_ts \|\| messageTs` and encodes `threadTs \|\| messageTs \|\| ""` — it falls back to the clicked message's `ts` even for DMs, so a DM button click spawns a phantom reply thread. Upstream's `handleMessageEvent` *does* empty-case DMs (`:2158`), but `handleBlockActions` does **not** — an upstream internal inconsistency. | Hard UX failure with no workaround (phantom "1 reply" threads on DM button clicks). We extend upstream's own DM-message convention to the block-action path. The resulting empty DM `thread_ts` is consumed by `fetch_messages` → now routed to `conversations.history` for empty `thread_ts` (see the #138 row below); the block-action fix should be contributed upstream (cf. PR #107's stream() divergence) to restore parity. |
+| Slack empty-DM `thread_ts` fetch routing (#138) | `fetch_messages` routes an empty (falsy) `thread_ts` — every top-level DM root, encoded `slack:Dxxx:` — to the channel-history path (`_fetch_channel_messages_forward` / `_fetch_channel_messages_backward`, both `conversations.history`) instead of `conversations.replies(ts="")`, preserving direction/limit/cursor. `fetch_message` likewise reads a single empty-`thread_ts` message via `conversations.history(channel, latest=message_id, inclusive=True, limit=1)` (mirroring the inner link-preview `fetch_message` at `slack/adapter.py:3293`). Non-empty `thread_ts` stays byte-identical on `conversations.replies`. | `fetchMessages` (`adapter-slack/src/index.ts:4135` → `fetchMessagesForward`/`Backward` `:4187`/`:4250`) and `fetchMessage` (`:4350`) call `conversations.replies({ ts: threadTs })` with **no** empty-`thread_ts` guard. With `ts=""` Slack returns no replies and the DM root context is lost. | Hard UX failure on **every** DM root: history/single-message fetches over a DM silently return nothing (or lose the root) because the DM root legitimately encodes `threadTs=""` (faithful to `_handle_message_event` / `handleMessageEvent`, "matches openDM subscriptions"). For a DM the channel **is** the conversation, so `conversations.history` is the correct source. This supersedes the "separate follow-up" noted in the #133/#137 row and covers DM message fetches **and** the #137 DM block-action consumer uniformly. Candidate to file upstream against vercel/chat (an empty-`thread_ts` guard in `fetchMessages`/`fetchMessage`); remove this divergence once upstream adds it. |
 | `GitHubAdapter.octokit` native client getter (vercel/chat#459, #478) | Not exposed | `get octokit(): Octokit` (plus deprecated `client` alias) returns the underlying Octokit — fixed instance in PAT/single-tenant App mode, per-installation client resolved from `AsyncLocalStorage` inside a webhook handler in multi-tenant mode | The Python adapter is hand-rolled over raw `aiohttp` (`_github_api_request`) with PyJWT for App JWTs and an installation-token cache; the `github` extra is `pyjwt[crypto]` only — there is no Octokit-equivalent object to return, and exposing the raw session or an invented facade under the name `octokit` would misrepresent the surface. Revisit if the adapter adopts an octokit-style SDK (e.g. `githubkit`) as an optional dependency per hazard #10's "prefer official SDKs" sub-rule; the getter (and the GitHub `fetch_subject` half of #459) ports cleanly then. |
 | `LinearAdapter.linear_client` native client getter (vercel/chat#459, #478) | Not exposed | `get linearClient(): LinearClient` (plus deprecated `client` alias) returns the `@linear/sdk` `LinearClient`, per-org from `AsyncLocalStorage` in multi-tenant OAuth mode | `@linear/sdk` is TypeScript-only and no official Linear Python SDK exists; the adapter issues GraphQL directly over `aiohttp` (`_graphql_query`) and already documents that stance. Nothing honest to put behind the name. Revisit only if Linear ships an official Python SDK (the Linear `fetch_subject` half of #459 is blocked on the same). |
 | `@chat-adapter/tests` adapter test kit (vercel/chat#470) | Not ported | New TS package with test utilities for adapter authors | Python already ships `chat_sdk.testing` (`MockAdapter`, `MockStateAdapter`, `create_test_message()`) covering the same surface for this repo's adapter tests; mirroring the TS kit verbatim would duplicate it. Revisit if upstream's kit grows capabilities ours lacks (e.g. recorded replay fixtures for third-party adapter authors). |

src/chat_sdk/adapters/slack/adapter.py

@@ -4317,15 +4317,21 @@ async def fetch_messages(self, thread_id: str, options: FetchOptions | None = No
         thread_ts = decoded.thread_ts
         direction = getattr(opts, "direction", "backward") or "backward"
         limit = getattr(opts, "limit", 100) if getattr(opts, "limit", 100) is not None else 100
+        cursor = getattr(opts, "cursor", None)
 
+        # Divergence (chat-sdk-python#138): a top-level DM root encodes an empty
+        # thread_ts (slack:Dxxx:). conversations.replies(ts="") returns no replies
+        # and loses the DM root context, so route empty thread_ts to the channel
+        # history path (conversations.history), where the channel *is* the
+        # conversation. Upstream's fetchMessages has no empty-thread_ts guard.
         try:
+            if not thread_ts:
+                if direction == "forward":
+                    return await self._fetch_channel_messages_forward(channel, limit, cursor)
+                return await self._fetch_channel_messages_backward(channel, limit, cursor)
             if direction == "forward":
-                return await self._fetch_messages_forward(
-                    channel, thread_ts, thread_id, limit, getattr(opts, "cursor", None)
-                )
-            return await self._fetch_messages_backward(
-                channel, thread_ts, thread_id, limit, getattr(opts, "cursor", None)
-            )
+                return await self._fetch_messages_forward(channel, thread_ts, thread_id, limit, cursor)
+            return await self._fetch_messages_backward(channel, thread_ts, thread_id, limit, cursor)
         except Exception as error:
             self._handle_slack_error(error)
 
@@ -4383,9 +4389,16 @@ async def fetch_message(self, thread_id: str, message_id: str) -> Message | None
 
         try:
             client = self._get_client()
-            result = await client.conversations_replies(
-                channel=channel, ts=thread_ts, oldest=message_id, inclusive=True, limit=1
-            )
+            # Divergence (chat-sdk-python#138): a DM root encodes an empty
+            # thread_ts, so conversations.replies(ts="") cannot locate the
+            # message. Fetch the single message from conversations.history
+            # instead (mirrors the link-preview fetch_message at ~3293).
+            if not thread_ts:
+                result = await client.conversations_history(channel=channel, latest=message_id, inclusive=True, limit=1)
+            else:
+                result = await client.conversations_replies(
+                    channel=channel, ts=thread_ts, oldest=message_id, inclusive=True, limit=1
+                )
             messages = result.get("messages", [])
             target = next((m for m in messages if m.get("ts") == message_id), None)
             if not target:

tests/test_slack_api.py

@@ -612,6 +612,180 @@ async def test_fetch_with_limit(self):
         # Should return at most limit messages
         assert len(result.messages) <= 10
 
+    @pytest.mark.asyncio
+    async def test_empty_dm_thread_ts_backward_uses_history_not_replies(self):
+        """A DM root (slack:Dxxx:) encodes thread_ts="" — backward fetch must
+        route to conversations.history (the channel IS the conversation), not
+        conversations.replies(ts="") which returns nothing for a DM (#138)."""
+        adapter, client, _ = await _init_adapter()
+        client.set_response(
+            "conversations_history",
+            {
+                "ok": True,
+                "messages": [
+                    {"ts": "1234567890.000002", "text": "DM 2", "user": "U2"},
+                    {"ts": "1234567890.000001", "text": "DM 1", "user": "U1"},
+                ],
+                "has_more": False,
+            },
+        )
+
+        result = await adapter.fetch_messages("slack:D999:")
+
+        # The DM root messages come back via conversations.history.
+        assert len(result.messages) == 2
+        history_calls = client.get_calls("conversations_history")
+        assert len(history_calls) == 1
+        assert history_calls[0]["kwargs"]["channel"] == "D999"
+        # conversations.replies must NOT be hit at all (esp. not with ts="").
+        replies_calls = client.get_calls("conversations_replies")
+        assert replies_calls == []
+
+    @pytest.mark.asyncio
+    async def test_empty_dm_thread_ts_forward_uses_history_not_replies(self):
+        """Forward direction over a DM root also routes to conversations.history,
+        preserving cursor/limit semantics, never conversations.replies(ts="")."""
+        adapter, client, _ = await _init_adapter()
+        client.set_response(
+            "conversations_history",
+            {
+                "ok": True,
+                "messages": [
+                    {"ts": "1234567890.000002", "text": "Newer", "user": "U2"},
+                    {"ts": "1234567890.000001", "text": "Older", "user": "U1"},
+                ],
+                "has_more": True,
+            },
+        )
+
+        result = await adapter.fetch_messages(
+            "slack:D999:",
+            FetchOptions(direction="forward", cursor="1234567890.000000", limit=50),
+        )
+
+        assert len(result.messages) == 2
+        history_calls = client.get_calls("conversations_history")
+        assert len(history_calls) == 1
+        assert history_calls[0]["kwargs"]["channel"] == "D999"
+        # Forward cursor maps to oldest= on conversations.history (channel-history path).
+        assert history_calls[0]["kwargs"]["oldest"] == "1234567890.000000"
+        assert history_calls[0]["kwargs"]["limit"] == 50
+        # has_more + slack messages → next_cursor from the newest ts.
+        assert result.next_cursor == "1234567890.000002"
+        assert client.get_calls("conversations_replies") == []
+
+    @pytest.mark.asyncio
+    async def test_non_empty_thread_ts_still_uses_replies_backward(self):
+        """Regression guard: a real thread root (non-empty thread_ts) MUST keep
+        using conversations.replies — the empty-DM routing must not over-trigger."""
+        adapter, client, _ = await _init_adapter()
+        client.set_response(
+            "conversations_replies",
+            {
+                "ok": True,
+                "messages": [
+                    {"ts": "1234567890.000001", "text": "Reply 1", "user": "U1"},
+                ],
+                "has_more": False,
+            },
+        )
+
+        result = await adapter.fetch_messages("slack:C123:1234567890.000000")
+
+        assert len(result.messages) == 1
+        replies_calls = client.get_calls("conversations_replies")
+        assert len(replies_calls) == 1
+        assert replies_calls[0]["kwargs"]["ts"] == "1234567890.000000"
+        # Channel-history path must NOT be used for a real thread.
+        assert client.get_calls("conversations_history") == []
+
+    @pytest.mark.asyncio
+    async def test_non_empty_thread_ts_still_uses_replies_forward(self):
+        """Regression guard (forward): non-empty thread_ts keeps conversations.replies."""
+        adapter, client, _ = await _init_adapter()
+        client.set_response(
+            "conversations_replies",
+            {
+                "ok": True,
+                "messages": [
+                    {"ts": "1234567890.000001", "text": "First", "user": "U1"},
+                ],
+                "response_metadata": {"next_cursor": "cur"},
+            },
+        )
+
+        result = await adapter.fetch_messages(
+            "slack:C123:1234567890.000000",
+            FetchOptions(direction="forward"),
+        )
+
+        assert len(result.messages) == 1
+        replies_calls = client.get_calls("conversations_replies")
+        assert len(replies_calls) == 1
+        assert replies_calls[0]["kwargs"]["ts"] == "1234567890.000000"
+        assert client.get_calls("conversations_history") == []
+
+
+# =============================================================================
+# fetchMessage (single) Tests
+# =============================================================================
+
+
+class TestFetchSingleMessage:
+    @pytest.mark.asyncio
+    async def test_non_empty_thread_ts_uses_replies(self):
+        """A single-message fetch on a real thread uses conversations.replies
+        (oldest=message_id) — byte-identical to the pre-#138 path."""
+        adapter, client, _ = await _init_adapter()
+        client.set_response(
+            "conversations_replies",
+            {
+                "ok": True,
+                "messages": [
+                    {"ts": "1234567890.000050", "text": "Target", "user": "U1"},
+                ],
+            },
+        )
+
+        msg = await adapter.fetch_message("slack:C123:1234567890.000000", "1234567890.000050")
+
+        assert msg is not None
+        assert msg.id == "1234567890.000050"
+        replies_calls = client.get_calls("conversations_replies")
+        assert len(replies_calls) == 1
+        assert replies_calls[0]["kwargs"]["ts"] == "1234567890.000000"
+        assert replies_calls[0]["kwargs"]["oldest"] == "1234567890.000050"
+        assert client.get_calls("conversations_history") == []
+
+    @pytest.mark.asyncio
+    async def test_empty_dm_thread_ts_uses_history(self):
+        """A single-message fetch on a DM root (empty thread_ts) reads from
+        conversations.history (latest=message_id), NOT conversations.replies(ts="")
+        which cannot locate the message (#138)."""
+        adapter, client, _ = await _init_adapter()
+        client.set_response(
+            "conversations_history",
+            {
+                "ok": True,
+                "messages": [
+                    {"ts": "1234567890.000050", "text": "DM message", "user": "U1"},
+                ],
+            },
+        )
+
+        msg = await adapter.fetch_message("slack:D999:", "1234567890.000050")
+
+        assert msg is not None
+        assert msg.id == "1234567890.000050"
+        history_calls = client.get_calls("conversations_history")
+        assert len(history_calls) == 1
+        assert history_calls[0]["kwargs"]["channel"] == "D999"
+        assert history_calls[0]["kwargs"]["latest"] == "1234567890.000050"
+        assert history_calls[0]["kwargs"]["inclusive"] is True
+        assert history_calls[0]["kwargs"]["limit"] == 1
+        # conversations.replies must NOT be called with ts="".
+        assert client.get_calls("conversations_replies") == []
+
 
 # =============================================================================
 # fetchThread Tests