fix(slack): don't thread block-action responses in DMs (supersedes #133)

.gitignore

@@ -20,3 +20,4 @@ uv.lock
 coverage.xml
 htmlcov/
 AGENTS.md
+.DS_Store

docs/UPSTREAM_SYNC.md

@@ -637,6 +637,7 @@ 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. |
 | `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

@@ -1842,10 +1842,15 @@ def _handle_block_actions(self, payload: dict[str, Any], options: WebhookOptions
 
         channel = (payload.get("channel") or {}).get("id") or (payload.get("container") or {}).get("channel_id")
         message_ts = (payload.get("message") or {}).get("ts") or (payload.get("container") or {}).get("message_ts")
+        # DMs have no threads: a button click on a top-level DM message must not
+        # thread the response. Mirror _handle_message_event's DM handling — keep a
+        # real in-DM thread_ts, but never fall back to the clicked message's own
+        # ts, which would spawn a phantom "1 reply" thread in the DM.
+        is_dm = bool(channel) and channel.startswith("D")
         thread_ts = (
             (payload.get("message") or {}).get("thread_ts")
             or (payload.get("container") or {}).get("thread_ts")
-            or message_ts
+            or ("" if is_dm else message_ts)
         )
 
         is_view_action = (payload.get("container") or {}).get("type") == "view"
@@ -1856,7 +1861,12 @@ def _handle_block_actions(self, payload: dict[str, Any], options: WebhookOptions
 
         thread_id = ""
         if channel and (thread_ts or message_ts):
-            thread_id = self.encode_thread_id(SlackThreadId(channel=channel, thread_ts=thread_ts or message_ts or ""))
+            thread_id = self.encode_thread_id(
+                SlackThreadId(
+                    channel=channel,
+                    thread_ts=thread_ts if thread_ts is not None else "",
+                )
+            )
 
         is_ephemeral = (payload.get("container") or {}).get("is_ephemeral") is True
         response_url = payload.get("response_url")

tests/test_slack_webhook.py

@@ -337,6 +337,78 @@ async def test_handles_block_actions(self):
         response = await adapter.handle_webhook(req)
         assert response["status"] == 200
 
+    @pytest.mark.asyncio
+    async def test_dm_block_action_does_not_thread(self):
+        """A click on a top-level DM message must resolve to a DM-root thread_id.
+
+        Regression: _handle_block_actions fell back to message_ts for thread_ts
+        in DMs, so HITL approval result cards posted via event.thread.post became
+        phantom "1 reply" threads in the DM. DMs have no threads — the ActionEvent
+        must carry an empty thread_ts, mirroring _handle_message_event.
+        """
+        adapter = _make_adapter()
+        chat = _make_mock_chat(_make_mock_state())
+        await adapter.initialize(chat)
+
+        req = self._make_interactive_req(
+            {
+                "type": "block_actions",
+                "user": {"id": "U123", "username": "testuser", "name": "Test User"},
+                "container": {"type": "message", "message_ts": "1234567890.123456", "channel_id": "D456"},
+                "channel": {"id": "D456", "name": "directmessage"},
+                "message": {"ts": "1234567890.123456"},  # top-level DM message: no thread_ts
+                "actions": [{"type": "button", "action_id": "approve_btn", "value": "approved"}],
+            }
+        )
+        response = await adapter.handle_webhook(req)
+        assert response["status"] == 200
+
+        chat.process_action.assert_called_once()
+        action_event = chat.process_action.call_args[0][0]
+        decoded = adapter.decode_thread_id(action_event.thread_id)
+        assert decoded.channel == "D456"
+        assert decoded.thread_ts == "", (
+            f"DM block action threaded under {decoded.thread_ts!r}; a top-level DM click "
+            "must resolve to a DM-root thread_id (empty thread_ts), else the approval "
+            "result card posts as a phantom reply thread."
+        )
+
+    @pytest.mark.asyncio
+    async def test_channel_block_action_threads_under_clicked_message(self):
+        """A click on a top-level channel message must thread under that message.
+
+        Counterpart to the DM case: channels DO have threads, so a button click on
+        a top-level (no thread_ts) channel message must fall back to the clicked
+        message's own ts so the response card threads under it. The DM-only guard
+        must not leak into channels.
+        """
+        adapter = _make_adapter()
+        chat = _make_mock_chat(_make_mock_state())
+        await adapter.initialize(chat)
+
+        req = self._make_interactive_req(
+            {
+                "type": "block_actions",
+                "user": {"id": "U123", "username": "testuser", "name": "Test User"},
+                "container": {"type": "message", "message_ts": "1234567890.123456", "channel_id": "C456"},
+                "channel": {"id": "C456", "name": "general"},
+                "message": {"ts": "1234567890.123456"},  # top-level channel message: no thread_ts
+                "actions": [{"type": "button", "action_id": "approve_btn", "value": "approved"}],
+            }
+        )
+        response = await adapter.handle_webhook(req)
+        assert response["status"] == 200
+
+        chat.process_action.assert_called_once()
+        action_event = chat.process_action.call_args[0][0]
+        decoded = adapter.decode_thread_id(action_event.thread_id)
+        assert decoded.channel == "C456"
+        assert decoded.thread_ts == "1234567890.123456", (
+            f"channel block action resolved to thread_ts {decoded.thread_ts!r}; a top-level "
+            "channel click must thread under the clicked message's own ts, not a DM-root "
+            "empty thread_ts."
+        )
+
     @pytest.mark.asyncio
     async def test_returns_400_for_missing_payload(self):
         adapter = _make_adapter()