From 32d99f511d91a8ccc890047ea0f92476526bdf7b Mon Sep 17 00:00:00 2001 From: tony-chinchill Date: Thu, 4 Jun 2026 17:17:33 -0700 Subject: [PATCH] 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()