From 32d99f511d91a8ccc890047ea0f92476526bdf7b Mon Sep 17 00:00:00 2001 From: tony-chinchill Date: Thu, 4 Jun 2026 17:17:33 -0700 Subject: [PATCH 1/4] fix(slack): don't thread block-action responses in DMs _handle_block_actions fell back to the clicked message's ts for thread_ts in DMs, so HITL approval result cards posted via event.thread.post became phantom "1 reply" threads in the DM. Mirror _handle_message_event's DM handling: keep a real in-DM thread_ts but never fall back to message_ts for a top-level DM click. Co-Authored-By: Claude Opus 4.8 (1M context) --- .DS_Store | Bin 0 -> 6148 bytes src/.DS_Store | Bin 0 -> 6148 bytes src/chat_sdk/.DS_Store | Bin 0 -> 6148 bytes src/chat_sdk/adapters/slack/adapter.py | 9 +++++-- tests/test_slack_webhook.py | 36 +++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 .DS_Store create mode 100644 src/.DS_Store create mode 100644 src/chat_sdk/.DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..fbad0b2d2b91180eb8af1601da15085bce3af4a1 GIT binary patch literal 6148 zcmeHKy-EW?5S}%siP{7~u)Ib^5h0z8aK^I|ZGyESmzW5K3;8MRuCf+v#TUo}_yYdK zLa|lQD)=1E?2gIqU1BMsJFxp*c4qdQ{gT_w0)WVO$~k~60A#QcIy2afF!EDN*aVL( zAPSn}q?#)PVZD)vv;$TFtH5tmfZkmP)}ac=&^F%R?ch-N)kEJeorEPE%Y1C!buV7; zw@iQeTYsab?G5{w36TEcg&K6AWUvDj7LU+tm_Fz(zx8Adv3cs@u#)jnMm-?6ECc4NN(Mf?hO@oi+SC*4z_~j6D8IogBDKmEvNrJ>Xh1v04o8<#2O~nS;juEP zAq}ypp$_AtjCw%UlfrqL5Ev@VUH7pL_UQb#OAlB5XQ{!!JLFAz4ZiszP70H_>I!nk z*S@%lCzS5hc~5UPcHdtlua>W?KRUwTUpXiK|7v(0#<=PrGRk!f}h7F@!6VK9G$f; uwjwq%ii;G=6zupomJf6k%j-KL#k!0hW28`~qF<%jKLVtRHLJkSDDVk9-Nds1 literal 0 HcmV?d00001 diff --git a/src/.DS_Store b/src/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..fe2507de4d3fcdf0f5bd093c706ed12b2c6d4218 GIT binary patch literal 6148 zcmeHKy-EW?5T4N#2R1ke{o9o zUeS_PbW0EPY5&W9yVDJyKE0KEwtdqqHvK|u>;1*m>&x?H*{}PS-)>d6hc(YKQiTKq z!9Xw&3eZ$Lu&5 z!UACn1zIS3iNO|*`Q(1taWJ%SVlO_}SNr0&GoF#2F17#K5fY{RMC|8x8@ z#Ug(k5~E-s82D!l@T6&*89vJI)-RtYcWpwuK@$$27ZBoci$5+DF6Tf literal 0 HcmV?d00001 diff --git a/src/chat_sdk/.DS_Store b/src/chat_sdk/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..e7b9aa5f6197e715d591a5c7eaf514bf5313ec35 GIT binary patch literal 6148 zcmeHKu}%U(5S>L7A~B&dmS?4nB$Re1IU5Tben9XH7{Mce{l)LGAhFb58$ZFsZ!obn zzM0(|w;Uu^#*mq0_wCKj?!0>!c32`Zt-H8MR41Y;8e@4I-4f$@ZUrkj4=*;d$CR#U zOp`&c*)Bw@Llsa3{+j~!0O(Lm9U4*Y_j{QPM?$vt{S2&P-isL~QPdqIU7l;4JsiBh zzML$Ni+>e2>Nne2&Yc;j-l7Thk#~$!CFV0Fn>UU7#rZs+-TkM@&71J?_82B}E~{cZ z9!4bC1dl0rKpl?-%~-wMY@WmCVdePCa_}PN436LXqOsy#V~d`{DF%L9i|rTax#(``?$=;-B66HP$rOYGNDE7 zRRLANS75_F*17(luD<{Ki}XnqPzBaX0aK3JQ46o+YHQ);xYh<}7ier8ms*r1=v+CL f0hi)EG%?HtJOG9cQ;YDx^pAkZpq(o4rwaT4Ckc?h literal 0 HcmV?d00001 diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index d1168ee1..8358a985 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -1605,10 +1605,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" @@ -1619,7 +1624,7 @@ 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)) is_ephemeral = (payload.get("container") or {}).get("is_ephemeral") is True response_url = payload.get("response_url") diff --git a/tests/test_slack_webhook.py b/tests/test_slack_webhook.py index 26387898..07fc2b63 100644 --- a/tests/test_slack_webhook.py +++ b/tests/test_slack_webhook.py @@ -337,6 +337,42 @@ 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_returns_400_for_missing_payload(self): adapter = _make_adapter() From 2b6c092c1c44804b8a1399d96377eb5275297e5e Mon Sep 17 00:00:00 2001 From: Patrick Date: Sun, 14 Jun 2026 09:54:08 +0000 Subject: [PATCH 2/4] fix(slack): restore thread_ts str-coalesce + drop committed .DS_Store Address two defects from local review of the DM block-action threading fix: 1. pyrefly bad-argument-type regression: the block-action handler replaced the established `thread_ts or message_ts or ""` coalesce with bare `thread_ts`, letting `str | None` flow into SlackThreadId(thread_ts: str). Restore the coalesce as `thread_ts or ""`, which both preserves the DM empty-thread_ts intent and guarantees `str` (thread_ts already folds in the message_ts fallback for channels). pyrefly src/ back to 0 errors. 2. Remove three committed .DS_Store binaries (./, src/, src/chat_sdk/) and add .DS_Store to .gitignore so they cannot return. Also add a channel-path counterpart test (test_channel_block_action_threads_under_clicked_message) so the DM-only guard is proven not to leak into channels, alongside the existing DM regression test. --- .DS_Store | Bin 6148 -> 0 bytes .gitignore | 1 + src/.DS_Store | Bin 6148 -> 0 bytes src/chat_sdk/.DS_Store | Bin 6148 -> 0 bytes src/chat_sdk/adapters/slack/adapter.py | 2 +- tests/test_slack_webhook.py | 36 +++++++++++++++++++++++++ 6 files changed, 38 insertions(+), 1 deletion(-) delete mode 100644 .DS_Store delete mode 100644 src/.DS_Store delete mode 100644 src/chat_sdk/.DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index fbad0b2d2b91180eb8af1601da15085bce3af4a1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKy-EW?5S}%siP{7~u)Ib^5h0z8aK^I|ZGyESmzW5K3;8MRuCf+v#TUo}_yYdK zLa|lQD)=1E?2gIqU1BMsJFxp*c4qdQ{gT_w0)WVO$~k~60A#QcIy2afF!EDN*aVL( zAPSn}q?#)PVZD)vv;$TFtH5tmfZkmP)}ac=&^F%R?ch-N)kEJeorEPE%Y1C!buV7; zw@iQeTYsab?G5{w36TEcg&K6AWUvDj7LU+tm_Fz(zx8Adv3cs@u#)jnMm-?6ECc4NN(Mf?hO@oi+SC*4z_~j6D8IogBDKmEvNrJ>Xh1v04o8<#2O~nS;juEP zAq}ypp$_AtjCw%UlfrqL5Ev@VUH7pL_UQb#OAlB5XQ{!!JLFAz4ZiszP70H_>I!nk z*S@%lCzS5hc~5UPcHdtlua>W?KRUwTUpXiK|7v(0#<=PrGRk!f}h7F@!6VK9G$f; uwjwq%ii;G=6zupomJf6k%j-KL#k!0hW28`~qF<%jKLVtRHLJkSDDVk9-Nds1 diff --git a/.gitignore b/.gitignore index 4c15006d..f4c55028 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ uv.lock coverage.xml htmlcov/ AGENTS.md +.DS_Store diff --git a/src/.DS_Store b/src/.DS_Store deleted file mode 100644 index fe2507de4d3fcdf0f5bd093c706ed12b2c6d4218..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKy-EW?5T4N#2R1ke{o9o zUeS_PbW0EPY5&W9yVDJyKE0KEwtdqqHvK|u>;1*m>&x?H*{}PS-)>d6hc(YKQiTKq z!9Xw&3eZ$Lu&5 z!UACn1zIS3iNO|*`Q(1taWJ%SVlO_}SNr0&GoF#2F17#K5fY{RMC|8x8@ z#Ug(k5~E-s82D!l@T6&*89vJI)-RtYcWpwuK@$$27ZBoci$5+DF6Tf diff --git a/src/chat_sdk/.DS_Store b/src/chat_sdk/.DS_Store deleted file mode 100644 index e7b9aa5f6197e715d591a5c7eaf514bf5313ec35..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKu}%U(5S>L7A~B&dmS?4nB$Re1IU5Tben9XH7{Mce{l)LGAhFb58$ZFsZ!obn zzM0(|w;Uu^#*mq0_wCKj?!0>!c32`Zt-H8MR41Y;8e@4I-4f$@ZUrkj4=*;d$CR#U zOp`&c*)Bw@Llsa3{+j~!0O(Lm9U4*Y_j{QPM?$vt{S2&P-isL~QPdqIU7l;4JsiBh zzML$Ni+>e2>Nne2&Yc;j-l7Thk#~$!CFV0Fn>UU7#rZs+-TkM@&71J?_82B}E~{cZ z9!4bC1dl0rKpl?-%~-wMY@WmCVdePCa_}PN436LXqOsy#V~d`{DF%L9i|rTax#(``?$=;-B66HP$rOYGNDE7 zRRLANS75_F*17(luD<{Ki}XnqPzBaX0aK3JQ46o+YHQ);xYh<}7ier8ms*r1=v+CL f0hi)EG%?HtJOG9cQ;YDx^pAkZpq(o4rwaT4Ckc?h diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index 8358a985..049eaae4 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -1624,7 +1624,7 @@ 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)) + thread_id = self.encode_thread_id(SlackThreadId(channel=channel, thread_ts=thread_ts or "")) is_ephemeral = (payload.get("container") or {}).get("is_ephemeral") is True response_url = payload.get("response_url") diff --git a/tests/test_slack_webhook.py b/tests/test_slack_webhook.py index 07fc2b63..d200ec12 100644 --- a/tests/test_slack_webhook.py +++ b/tests/test_slack_webhook.py @@ -373,6 +373,42 @@ async def test_dm_block_action_does_not_thread(self): "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() From 55f9397d08804d9308a04addda7891058170487b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 08:05:29 +0000 Subject: [PATCH 3/4] fix(slack): use explicit None-coalesce for DM thread_ts at encode time Per CLAUDE.md port rule, prefer 'x if x is not None else default' over 'x or default'. Functionally identical here (only falsy value is the intended empty string) and keeps the SlackThreadId(thread_ts: str) guarantee that pyrefly checks. --- src/chat_sdk/adapters/slack/adapter.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index 049eaae4..99c7118b 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -1624,7 +1624,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 "")) + 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") From b29025137c75b3b3bf5ac1ac985c0c24eafccd80 Mon Sep 17 00:00:00 2001 From: patrick-chinchill Date: Wed, 17 Jun 2026 02:24:38 -0700 Subject: [PATCH 4/4] docs(sync): record Slack DM block-action threading divergence (#133/#137) Upstream handleBlockActions falls back to messageTs even for DMs (phantom thread); we empty-case DMs to match upstream's own handleMessageEvent convention (index.ts:2158). Documented per the project divergence rule; cf. PR #107. --- docs/UPSTREAM_SYNC.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/UPSTREAM_SYNC.md b/docs/UPSTREAM_SYNC.md index 4269a08f..cd740d20 100644 --- a/docs/UPSTREAM_SYNC.md +++ b/docs/UPSTREAM_SYNC.md @@ -635,6 +635,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. | ### Platform-specific gaps