fix(slack): honor reply_in_thread=false for top-level channel messages
Top-level channel messages arrive at _resolve_thread_ts with metadata.thread_id set to the message's own ts, because the inbound handler in _handle_message_event uses 'event.ts' as a session-keying fallback when event.thread_ts is absent. That made metadata alone insufficient to distinguish a real thread reply from a top-level message, so reply_in_thread=false only took effect in DMs. Use reply_to (== incoming message_id == ts for top-level messages) as the tiebreaker: when metadata.thread_id == reply_to the 'thread' is the synthetic session-keying fallback, not a real parent, so we reply directly in the channel. Real thread replies (reply_to != thread_id) still resolve to the parent thread and preserve conversation context. Closes #9268.
This commit is contained in:
@@ -450,8 +450,18 @@ class SlackAdapter(BasePlatformAdapter):
|
|||||||
"""
|
"""
|
||||||
# When reply_in_thread is disabled (default: True for backward compat),
|
# When reply_in_thread is disabled (default: True for backward compat),
|
||||||
# only thread messages that are already part of an existing thread.
|
# only thread messages that are already part of an existing thread.
|
||||||
|
# For top-level channel messages, the inbound handler sets
|
||||||
|
# metadata.thread_id to the message's own ts as a session-keying
|
||||||
|
# fallback (see the `thread_ts = event.get("thread_ts") or ts` branch),
|
||||||
|
# so metadata alone can't distinguish a real thread reply from a
|
||||||
|
# top-level message. reply_to is the incoming message's own id, so
|
||||||
|
# when thread_id == reply_to the "thread" is synthetic and we reply
|
||||||
|
# directly in the channel instead.
|
||||||
if not self.config.extra.get("reply_in_thread", True):
|
if not self.config.extra.get("reply_in_thread", True):
|
||||||
existing_thread = (metadata or {}).get("thread_id") or (metadata or {}).get("thread_ts")
|
md = metadata or {}
|
||||||
|
existing_thread = md.get("thread_id") or md.get("thread_ts")
|
||||||
|
if existing_thread and reply_to and existing_thread == reply_to:
|
||||||
|
existing_thread = None
|
||||||
return existing_thread or None
|
return existing_thread or None
|
||||||
|
|
||||||
if metadata:
|
if metadata:
|
||||||
|
|||||||
@@ -334,7 +334,19 @@ def test_config_bridges_slack_reply_in_thread(monkeypatch, tmp_path):
|
|||||||
|
|
||||||
adapter = SlackAdapter(slack_config)
|
adapter = SlackAdapter(slack_config)
|
||||||
assert adapter._resolve_thread_ts(reply_to="171.000", metadata={}) is None
|
assert adapter._resolve_thread_ts(reply_to="171.000", metadata={}) is None
|
||||||
|
|
||||||
|
# Top-level channel messages arrive with metadata.thread_id == reply_to
|
||||||
|
# because the inbound handler uses event.ts as a session-keying fallback.
|
||||||
|
# Those must be treated as non-threaded so reply_in_thread=false takes
|
||||||
|
# effect in channels, not just DMs.
|
||||||
assert adapter._resolve_thread_ts(
|
assert adapter._resolve_thread_ts(
|
||||||
reply_to="171.000",
|
reply_to="171.000",
|
||||||
metadata={"thread_id": "171.000"},
|
metadata={"thread_id": "171.000"},
|
||||||
|
) is None
|
||||||
|
|
||||||
|
# Real thread replies (reply_to differs from thread parent) must still
|
||||||
|
# resolve to the parent thread so conversation context is preserved.
|
||||||
|
assert adapter._resolve_thread_ts(
|
||||||
|
reply_to="171.500",
|
||||||
|
metadata={"thread_id": "171.000"},
|
||||||
) == "171.000"
|
) == "171.000"
|
||||||
|
|||||||
Reference in New Issue
Block a user