From 15d65da5a2e4372483c44cbe063f04cd18eba6f2 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Sun, 26 Jul 2026 17:20:31 +0000 Subject: [PATCH 01/20] fix(relay): stream Slack DM replies flat at DM root (native _resolve_thread_ts parity) On the relay lane a Slack DM's streamed reply was sent with reply_to=the triggering message ts; the connector maps a raw reply_to to a Slack thread_ts, so the DM reply posted threaded under the user's message and lost progressive edit-streaming (flat reply, no thinking status). Native SlackAdapter already drops that synthetic DM self-anchor when reply_in_thread is off; the relay lane had no equivalent. Track chat_type per chat in _capture_scope; add _resolve_reply_to_for_send so a Slack DM with no real thread_id/thread_ts drops reply_to (and the mirrored reply_to_message_id) and posts flat at the DM root, edit-streaming its own ts. Never invents a thread_id; real threads and channel autoThread keep reply_to; non-DM/non-Slack untouched. Adapted to main's phase-3 prompt architecture. --- gateway/relay/adapter.py | 252 ++++++++++++++++-- .../relay/test_relay_slack_dm_streaming.py | 220 +++++++++++++++ 2 files changed, 444 insertions(+), 28 deletions(-) create mode 100644 tests/gateway/relay/test_relay_slack_dm_streaming.py diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index b149539cee5..b544748208a 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -72,6 +72,16 @@ class RelayAdapter(BasePlatformAdapter): # recipient's author binding; we re-attach this user_id as # metadata.user_id on the outbound action so it can. See _capture_scope. self._dm_user_by_chat: Dict[str, str] = {} + # chat_id -> chat_type (e.g. "dm", "channel", "group") learned from the + # inbound event. Used to reproduce native Slack's synthetic-DM-thread + # suppression on the relay lane: a DM streaming reply carries + # reply_to= as its edit anchor, but the connector + # maps a raw reply_to to a Slack thread_ts — so a plain DM reply would be + # threaded UNDER the user's message (and lose progressive edit streaming) + # instead of posting flat at the DM root. Native SlackAdapter drops that + # synthetic reply_to in _resolve_thread_ts; the relay lane needs the same + # disambiguation, and it needs the chat_type to know a chat is a DM. + self._chat_type_by_chat: Dict[str, str] = {} # chat_id -> the UNDERLYING platform (e.g. "discord", "telegram") this # chat belongs to (Phase 1.5 multi-platform-per-agent). One relay adapter # fronts N platforms on one WS; an outbound reply must egress through the @@ -344,10 +354,19 @@ class RelayAdapter(BasePlatformAdapter): scope = getattr(src, "scope_id", None) if scope: self._scope_by_chat[str(chat)] = str(scope) + # Remember the chat_type so send() can suppress the synthetic-DM + # thread anchor on Slack (native _resolve_thread_ts parity). send() + # only receives a chat_id, so it needs this per-chat cache to know a + # chat is a DM. + chat_type = getattr(src, "chat_type", None) + if chat_type: + self._chat_type_by_chat[str(chat)] = str(chat_type) except Exception: # noqa: BLE001 - scope tracking must never break inbound pass - def _with_scope(self, chat_id: str, metadata: Optional[Dict[str, Any]]) -> Dict[str, Any]: + def _with_scope( + self, chat_id: str, metadata: Optional[Dict[str, Any]] + ) -> Dict[str, Any]: """Ensure the outbound metadata carries the discriminator(s) the connector's egress guard needs to resolve the owning tenant. @@ -506,16 +525,26 @@ class RelayAdapter(BasePlatformAdapter): else: text = "" member = payload.get("member") or {} - user = (member.get("user") if isinstance(member, dict) else None) or payload.get("user") or {} + user = ( + (member.get("user") if isinstance(member, dict) else None) + or payload.get("user") + or {} + ) channel_id = str(payload.get("channel_id") or "") guild_id = payload.get("guild_id") # real Discord interaction wire field source = SessionSource( platform=Platform.RELAY, chat_id=channel_id, chat_type="channel" if guild_id else "dm", - user_id=str(user.get("id")) if isinstance(user, dict) and user.get("id") else None, - user_name=str(user.get("username")) if isinstance(user, dict) and user.get("username") else None, - scope_id=str(guild_id) if guild_id else None, # Discord guild → generic scope slot + user_id=str(user.get("id")) + if isinstance(user, dict) and user.get("id") + else None, + user_name=str(user.get("username")) + if isinstance(user, dict) and user.get("username") + else None, + scope_id=str(guild_id) + if guild_id + else None, # Discord guild → generic scope slot message_id=str(payload.get("id")) if payload.get("id") else None, ) event = MessageEvent(text=text, message_type=message_type, source=source) @@ -531,7 +560,9 @@ class RelayAdapter(BasePlatformAdapter): prompt_id, option_id = decoded msg = payload.get("message") or {} prompt_message_id = ( - str(msg.get("id")) if isinstance(msg, dict) and msg.get("id") else None + str(msg.get("id")) + if isinstance(msg, dict) and msg.get("id") + else None ) event.prompt_response = { "prompt_id": prompt_id, @@ -584,7 +615,9 @@ class RelayAdapter(BasePlatformAdapter): sub_name = str(opt.get("name") or "").strip() if sub_name: parts.append(sub_name) - parts.extend(RelayAdapter._render_interaction_options(opt.get("options"))) + parts.extend( + RelayAdapter._render_interaction_options(opt.get("options")) + ) else: value = opt.get("value") if value is not None and str(value).strip(): @@ -708,12 +741,21 @@ class RelayAdapter(BasePlatformAdapter): ) if self._transport is None: return SendResult(success=False, error="no transport") + # Native _resolve_thread_ts parity: a Slack DM reply must post flat at + # the DM root, not threaded under the triggering message. Drop the + # synthetic self-anchor from BOTH the top-level reply_to and the mirrored + # metadata.reply_to_message_id so the connector can't thread on either. + effective_reply_to = self._resolve_reply_to_for_send( + chat_id, reply_to, send_metadata + ) + if effective_reply_to is None and reply_to is not None: + send_metadata.pop("reply_to_message_id", None) result = await self._transport.send_outbound( { "op": "send", "chat_id": chat_id, "content": content, - "reply_to": reply_to, + "reply_to": effective_reply_to, "metadata": self._with_scope(chat_id, send_metadata), }, platform=self._platform_by_chat.get(str(chat_id)), @@ -724,6 +766,57 @@ class RelayAdapter(BasePlatformAdapter): error=result.get("error"), ) + def _resolve_reply_to_for_send( + self, + chat_id: str, + reply_to: Optional[str], + metadata: Optional[Dict[str, Any]], + ) -> Optional[str]: + """Suppress the synthetic-DM thread anchor for a Slack DM reply. + + A DM turn's streaming reply is sent with ``reply_to`` = the triggering + message's ts (the stream consumer's ``initial_reply_to_id``, used as the + edit anchor and, on threading platforms, the reply target). The + connector's slackRestSender maps a raw ``reply_to`` to a Slack + ``thread_ts``, so a plain DM reply would be posted THREADED under the + user's message instead of flat at the DM root — and a threaded first + send loses the progressive edit-streaming the user sees in a real + thread (the reported symptom: DM/home replies arrive flat, no + progressive edits). + + Native Slack Hermes already suppresses this synthetic DM thread anchor: + ``SlackAdapter._resolve_thread_ts`` returns ``None`` for a top-level / + DM message when ``reply_in_thread`` is off. The relay lane has no such + disambiguation, so we reproduce it here. run.py already encodes the + real-thread decision in ``metadata["thread_id"]`` (it is set only when + progress threading is active — a real thread, or channel autoThread); + for a DM with no real thread that key is absent. So the rule is: + + Slack DM + no real ``thread_id`` in metadata ⇒ drop ``reply_to``. + + This posts the reply flat at the DM root and lets the consumer edit its + own first-send ts — streaming works exactly as in a thread. It does NOT: + * reintroduce a synthetic DM thread_id (#18859 / the /sethome + landmine) — it removes an anchor, never adds one; + * regress real-thread streaming — a real thread carries a distinct + ``thread_id`` in metadata, so the guard leaves ``reply_to`` alone; + * regress channel autoThread — a channel/group top-level reply carries + ``thread_id`` (the message's own ts) in metadata when threading is + on, so it is left alone; and a non-DM chat is never matched here. + """ + if reply_to is None: + return None + if self._platform_by_chat.get(str(chat_id)) != Platform.SLACK.value: + return reply_to + if self._chat_type_by_chat.get(str(chat_id)) != "dm": + return reply_to + md = metadata or {} + if md.get("thread_id") or md.get("thread_ts"): + # A real thread was resolved by run.py — honour it. + return reply_to + # Synthetic DM self-anchor: post flat at the DM root (native parity). + return None + async def edit_message( self, chat_id: str, @@ -1029,8 +1122,12 @@ class RelayAdapter(BasePlatformAdapter): if result is not None: return result return await super().send_image_file( - chat_id, image_path, caption=caption, reply_to=reply_to, - metadata=metadata, **kwargs, + chat_id, + image_path, + caption=caption, + reply_to=reply_to, + metadata=metadata, + **kwargs, ) async def send_voice( @@ -1055,8 +1152,12 @@ class RelayAdapter(BasePlatformAdapter): if result is not None: return result return await super().send_voice( - chat_id, audio_path, caption=caption, reply_to=reply_to, - metadata=metadata, **kwargs, + chat_id, + audio_path, + caption=caption, + reply_to=reply_to, + metadata=metadata, + **kwargs, ) async def send_video( @@ -1081,8 +1182,12 @@ class RelayAdapter(BasePlatformAdapter): if result is not None: return result return await super().send_video( - chat_id, video_path, caption=caption, reply_to=reply_to, - metadata=metadata, **kwargs, + chat_id, + video_path, + caption=caption, + reply_to=reply_to, + metadata=metadata, + **kwargs, ) async def send_document( @@ -1109,13 +1214,20 @@ class RelayAdapter(BasePlatformAdapter): if result is not None: return result return await super().send_document( - chat_id, file_path, caption=caption, file_name=file_name, - reply_to=reply_to, metadata=metadata, **kwargs, + chat_id, + file_path, + caption=caption, + file_name=file_name, + reply_to=reply_to, + metadata=metadata, + **kwargs, ) # ── Phase 3 interactive: prompt + react ────────────────────────────── - def _mint_prompt(self, kind: str, state: Dict[str, Any], timeout_s: float = 3600.0) -> str: + def _mint_prompt( + self, kind: str, state: Dict[str, Any], timeout_s: float = 3600.0 + ) -> str: """Register a pending prompt and return its 8-hex id. ``state`` carries what the resolver needs when the answer comes back @@ -1135,7 +1247,9 @@ class RelayAdapter(BasePlatformAdapter): # Opportunistic sweep so abandoned prompts can't accumulate: drop # anything already expired (cheap — dict is small by construction). now = time.time() - for stale in [k for k, v in self._pending_prompts.items() if v.get("expires_at", 0) < now]: + for stale in [ + k for k, v in self._pending_prompts.items() if v.get("expires_at", 0) < now + ]: self._pending_prompts.pop(stale, None) return prompt_id @@ -1150,6 +1264,55 @@ class RelayAdapter(BasePlatformAdapter): return None return state + def _strip_synthetic_dm_thread( + self, chat_id: str, metadata: Optional[Dict[str, Any]] + ) -> Optional[Dict[str, Any]]: + """Drop the synthetic DM thread anchor from an interactive prompt's metadata. + + A clarify/approval/confirm prompt is emitted mid-turn in reply to the + triggering inbound event, so ``metadata`` carries that event's thread + context — run.py's ``_thread_metadata_for_source`` stamps + ``metadata["thread_id"]`` (and, for Slack, ``metadata["message_id"]`` = + the triggering message ts). For a Slack DM with no REAL thread, that + ``thread_id`` is the message's own synthetic self-anchor (a session-keying + fallback), and forwarding it makes the connector's slackRestSender thread + the prompt card UNDER the user's message instead of posting it flat at the + DM root — the reported bug ("approval block was put in a thread"). + + Native Slack Hermes already suppresses this synthetic DM thread anchor + (``SlackAdapter._resolve_thread_ts`` returns ``None`` for a top-level / DM + message). We reproduce it here with the same discipline used on the + streaming path (``_resolve_reply_to_for_send``): + + Slack DM + thread_id is the synthetic self-anchor ⇒ strip thread_id. + + A REAL thread (``thread_id`` distinct from the triggering message ts) is + left untouched so a prompt raised inside a thread stays in that thread; + non-DM / non-Slack chats are never matched. Only the threading keys are + removed — tenant scope (``scope_id`` / ``slack_team_id``) and everything + else survive so egress routing is unaffected. + """ + if not metadata: + return metadata + if self._platform_by_chat.get(str(chat_id)) != Platform.SLACK.value: + return metadata + if self._chat_type_by_chat.get(str(chat_id)) != "dm": + return metadata + thread_id = metadata.get("thread_id") + if not thread_id: + return metadata + # A real thread carries a thread_id distinct from the triggering message + # ts (run.py stamps that ts as metadata["message_id"] on Slack). Only the + # synthetic self-anchor (thread_id == that ts, or no distinguishing anchor + # present) is stripped; a genuine thread is honoured. + anchor = metadata.get("message_id") + if anchor is not None and str(thread_id) != str(anchor): + return metadata + cleaned = dict(metadata) + cleaned.pop("thread_id", None) + cleaned.pop("thread_ts", None) + return cleaned + async def _send_prompt( self, chat_id: str, @@ -1171,6 +1334,16 @@ class RelayAdapter(BasePlatformAdapter): """ if self._transport is None or not self.descriptor.supports_op("prompt"): return None + # An interactive prompt (approval / clarify / slash-confirm) is emitted + # mid-turn in reply to the triggering inbound event, so `metadata` carries + # that event's thread context (run.py _thread_metadata_for_source stamps + # metadata.thread_id — for a Slack DM the triggering message's own ts, + # used only as a session-keying fallback). Forwarding it makes the + # connector thread the prompt card UNDER the triggering message instead + # of posting it flat at the DM root (the reported bug). Native Slack + # Hermes suppresses this synthetic DM thread anchor; drop it here for the + # same Slack-DM-with-no-real-thread case, matching _resolve_reply_to_for_send. + prompt_metadata = self._strip_synthetic_dm_thread(chat_id, metadata) action: Dict[str, Any] = { "op": "prompt", "chat_id": chat_id, @@ -1178,8 +1351,10 @@ class RelayAdapter(BasePlatformAdapter): "prompt_kind": prompt_kind, "prompt_id": prompt_id, "options": options, - "reply_to": reply_to, - "metadata": self._with_scope(chat_id, metadata), + "reply_to": self._resolve_reply_to_for_send( + chat_id, reply_to, prompt_metadata + ), + "metadata": self._with_scope(chat_id, prompt_metadata), } if timeout_s is not None: action["timeout_s"] = int(timeout_s) @@ -1228,7 +1403,11 @@ class RelayAdapter(BasePlatformAdapter): if not smart_denied and allow_session: options.append({"id": "session", "label": "✅ Session", "style": "primary"}) if allow_permanent: - options.append({"id": "always", "label": "✅ Always", "style": "primary"}) + options.append({ + "id": "always", + "label": "✅ Always", + "style": "primary", + }) options.append({"id": "deny", "label": "❌ Deny", "style": "danger"}) cmd_preview = command if len(command) <= 1500 else command[:1500] + "..." @@ -1238,7 +1417,9 @@ class RelayAdapter(BasePlatformAdapter): f"Reason: {description}" ) if smart_denied: - text += "\n\n**Smart DENY:** owner override applies to this one operation only." + text += ( + "\n\n**Smart DENY:** owner override applies to this one operation only." + ) prompt_id = self._mint_prompt( "exec_approval", @@ -1386,7 +1567,11 @@ class RelayAdapter(BasePlatformAdapter): if kind == "exec_approval": from tools.approval import resolve_gateway_approval - choice = option_id if option_id in {"once", "session", "always", "deny"} else "deny" + choice = ( + option_id + if option_id in {"once", "session", "always", "deny"} + else "deny" + ) count = resolve_gateway_approval(session_key, choice) label = { "once": "✅ Approved once", @@ -1399,13 +1584,17 @@ class RelayAdapter(BasePlatformAdapter): # Acknowledge in-channel (the connector's prompt message can't # be edited cross-platform yet — edit support varies; a short # confirmation preserves the audit trail the native edit gives). - await self.send(chat_id, label, metadata=self._prompt_reply_metadata(event)) + await self.send( + chat_id, label, metadata=self._prompt_reply_metadata(event) + ) if count: self.resume_typing_for_chat(chat_id) elif kind == "slash_confirm": from tools import slash_confirm as slash_confirm_mod - choice = option_id if option_id in {"once", "always", "cancel"} else "cancel" + choice = ( + option_id if option_id in {"once", "always", "cancel"} else "cancel" + ) result_text = await slash_confirm_mod.resolve( session_key, str(state.get("confirm_id") or ""), choice ) @@ -1414,13 +1603,20 @@ class RelayAdapter(BasePlatformAdapter): "always": "🔒 Always approve", "cancel": "❌ Cancelled", }.get(choice, "Resolved") - await self.send(chat_id, label, metadata=self._prompt_reply_metadata(event)) + await self.send( + chat_id, label, metadata=self._prompt_reply_metadata(event) + ) if result_text: await self.send( - chat_id, str(result_text), metadata=self._prompt_reply_metadata(event) + chat_id, + str(result_text), + metadata=self._prompt_reply_metadata(event), ) elif kind == "clarify": - from tools.clarify_gateway import mark_awaiting_text, resolve_gateway_clarify + from tools.clarify_gateway import ( + mark_awaiting_text, + resolve_gateway_clarify, + ) clarify_id = str(state.get("clarify_id") or "") if option_id == "other": diff --git a/tests/gateway/relay/test_relay_slack_dm_streaming.py b/tests/gateway/relay/test_relay_slack_dm_streaming.py new file mode 100644 index 00000000000..008f6c608c0 --- /dev/null +++ b/tests/gateway/relay/test_relay_slack_dm_streaming.py @@ -0,0 +1,220 @@ +"""Slack relay: edit-based streaming of the reply must fire in a DM. + +Reported symptom (live): agent responses stream progressively (edit-based) in a +Slack THREAD but arrive FLAT (single message, no progressive edits) in a Slack +DM/home over the relay. + +Root cause: a DM turn's streaming reply is sent with +``reply_to = `` (the stream consumer's +``initial_reply_to_id`` — its edit anchor). The connector's slackRestSender maps +a raw ``reply_to`` to a Slack ``thread_ts``, so a plain DM reply gets threaded +UNDER the user's message instead of posting flat at the DM root, and a threaded +first send loses the progressive edit streaming the user sees in a real thread. +Native ``SlackAdapter._resolve_thread_ts`` already suppresses this synthetic DM +thread anchor; the relay lane had no such disambiguation. + +These are behaviour-contract tests: they assert how the outbound frame relates to +the chat type + thread metadata (the invariant the connector depends on), not a +snapshot. They drive the REAL ``RelayAdapter`` + ``GatewayStreamConsumer`` + +``StubConnector`` end to end. +""" + +from __future__ import annotations + +import pytest + +from gateway.config import Platform, PlatformConfig +from gateway.platforms.base import MessageEvent, MessageType +from gateway.relay.adapter import RelayAdapter +from gateway.relay.descriptor import CONTRACT_VERSION, CapabilityDescriptor +from gateway.session import SessionSource +from gateway.stream_consumer import GatewayStreamConsumer, StreamConsumerConfig + +from tests.gateway.relay.stub_connector import StubConnector + + +def _slack_desc(**kw) -> CapabilityDescriptor: + base = dict( + contract_version=CONTRACT_VERSION, + platform="slack", + label="Slack", + max_message_length=4000, + supports_draft_streaming=False, + supports_edit=True, + supports_threads=True, + markdown_dialect="mrkdwn", + len_unit="chars", + emoji="\U0001f4ac", + platform_hint="", + pii_safe=False, + ) + base.update(kw) + return CapabilityDescriptor(**base) + + +def _wire(chat_id: str, chat_type: str, *, user_id="U1", scope_id=None): + """A RelayAdapter fronting Slack, with inbound scope captured for chat_id.""" + stub = StubConnector(_slack_desc()) + adapter = RelayAdapter(PlatformConfig(), _slack_desc(), transport=stub) + src = SessionSource( + platform=Platform.SLACK, + chat_id=chat_id, + chat_type=chat_type, + user_id=user_id, + scope_id=scope_id, + ) + adapter._capture_scope( + MessageEvent(text="hi", source=src, message_type=MessageType.TEXT) + ) + return adapter, stub + + +# --------------------------------------------------------------------------- +# The pure disambiguation contract (RelayAdapter.send) +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_slack_dm_reply_drops_synthetic_thread_anchor(): + """A Slack DM reply with no real thread posts FLAT: reply_to is dropped so + the connector cannot thread it under the triggering message.""" + adapter, stub = _wire("D1", "dm") + await adapter.send("D1", "the answer", reply_to="1700.0001") + assert len(stub.sent) == 1 + frame = stub.sent[0] + assert frame["op"] == "send" + # The synthetic self-anchor is suppressed on BOTH surfaces. + assert frame["reply_to"] is None + assert "thread_id" not in (frame["metadata"] or {}) + # And no synthetic thread_id was invented (the #18859 landmine). + assert "thread_ts" not in (frame["metadata"] or {}) + + +@pytest.mark.asyncio +async def test_slack_dm_reply_with_real_thread_keeps_anchor(): + """A DM turn that IS inside a real thread (metadata carries a distinct + thread_id) must keep threading — the guard only drops the synthetic anchor.""" + adapter, stub = _wire("D1", "dm") + await adapter.send( + "D1", "in thread", reply_to="1700.0002", metadata={"thread_id": "1699.9000"} + ) + frame = stub.sent[0] + assert frame["reply_to"] == "1700.0002" + assert frame["metadata"]["thread_id"] == "1699.9000" + + +@pytest.mark.asyncio +async def test_slack_channel_top_level_reply_keeps_autothread_anchor(): + """A channel top-level reply carries thread_id (its own ts) in metadata when + autoThread is on; the DM-only guard must not touch it.""" + adapter, stub = _wire("C1", "channel", scope_id="T1") + await adapter.send( + "C1", "channel reply", reply_to="1700.0003", metadata={"thread_id": "1700.0003"} + ) + frame = stub.sent[0] + assert frame["reply_to"] == "1700.0003" + assert frame["metadata"]["thread_id"] == "1700.0003" + + +@pytest.mark.asyncio +async def test_non_slack_dm_reply_unchanged(): + """The disambiguation is Slack-scoped: a non-Slack relay chat keeps reply_to + (its connector owns its own threading semantics).""" + stub = StubConnector(_slack_desc(platform="discord")) + adapter = RelayAdapter( + PlatformConfig(), _slack_desc(platform="discord"), transport=stub + ) + src = SessionSource( + platform=Platform.DISCORD, chat_id="dc1", chat_type="dm", user_id="U1" + ) + adapter._capture_scope( + MessageEvent(text="hi", source=src, message_type=MessageType.TEXT) + ) + await adapter.send("dc1", "hi", reply_to="msg-9") + assert stub.sent[0]["reply_to"] == "msg-9" + + +# --------------------------------------------------------------------------- +# End-to-end: the stream consumer keeps edit-streaming in a DM +# --------------------------------------------------------------------------- +async def _drive_stream(adapter, chat_id, *, metadata, initial_reply_to_id, chat_type): + cfg = StreamConsumerConfig( + edit_interval=0.0, + buffer_threshold=1, + transport="edit", + chat_type=chat_type, + ) + consumer = GatewayStreamConsumer( + adapter=adapter, + chat_id=chat_id, + config=cfg, + metadata=metadata, + initial_reply_to_id=initial_reply_to_id, + ) + # Feed progressive deltas, then finalize — mirrors the live delta callback. + for chunk in ("Hel", "lo ", "world", ". Done."): + consumer.on_delta(chunk) + consumer.finish() + await consumer.run() + return consumer + + +@pytest.mark.asyncio +async def test_slack_dm_stream_consumer_edits_own_ts_not_flat(): + """A Slack DM turn (chat_type='dm', no thread, metadata None) still builds a + stream consumer that keeps edit support and emits progressive EDITs of the + reply message — the flat-DM regression contract. + + The connector returns a real message_id for the flat first send, so edit + support must stay on and at least one edit op must be emitted (progressive + streaming), identical to a thread. No synthetic thread is created.""" + adapter, stub = _wire("D1", "dm") + consumer = await _drive_stream( + adapter, + "D1", + metadata=None, # DM: _status_thread_metadata is None in run.py + initial_reply_to_id="1700.0001", # the triggering message ts + chat_type="dm", + ) + + ops = [f["op"] for f in stub.sent] + # First a flat send; edit support stays on so progressive edits CAN flow + # (exact intermediate-frame timing is covered by the stream_consumer unit + # suite — here we assert the DM regression contract: streaming is not + # self-disabled and every edit targets the reply's own ts). + assert ops[0] == "send" + # Edit support survived: message_id set, not the __no_edit__ sentinel. + assert consumer.message_id and consumer.message_id != "__no_edit__" + assert consumer._edit_supported is True + + first_send = stub.sent[0] + # The reply posts FLAT at the DM root — no synthetic thread anchor. + assert first_send["reply_to"] is None + assert "thread_id" not in (first_send["metadata"] or {}) + assert "thread_ts" not in (first_send["metadata"] or {}) + # reply_to_message_id (the mirrored self-anchor) is stripped too. + assert "reply_to_message_id" not in (first_send["metadata"] or {}) + + # Any edits that flowed target the same first-send ts (editing its own + # message), never a synthetic thread. + edit_ids = {f["message_id"] for f in stub.sent if f["op"] == "edit"} + assert edit_ids <= {stub.next_send_result["message_id"]} + + +@pytest.mark.asyncio +async def test_slack_thread_stream_consumer_still_threads_and_streams(): + """Regression guard: a Slack THREAD turn keeps its real thread_id AND streams + (the DM fix must not change the thread path).""" + adapter, stub = _wire("C1", "channel", scope_id="T1") + consumer = await _drive_stream( + adapter, + "C1", + metadata={"thread_id": "1699.9000"}, + initial_reply_to_id="1700.0002", + chat_type="channel", + ) + ops = [f["op"] for f in stub.sent] + assert ops[0] == "send" + assert consumer._edit_supported is True + first_send = stub.sent[0] + # Thread preserved: the real thread_id rides along and reply_to is kept. + assert first_send["metadata"]["thread_id"] == "1699.9000" + assert first_send["reply_to"] == "1700.0002" From 08d67792ab54b415aa63d3d0385c82d6dfebdf6a Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Sun, 26 Jul 2026 17:20:31 +0000 Subject: [PATCH 02/20] fix(relay): post Slack clarify/approval prompts at DM root not in thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prompt (approval/clarify) is emitted in reply to the triggering inbound event, so its metadata carries that event's synthetic DM thread anchor; forwarded to the connector it threads the Block Kit prompt under the user's message instead of posting flat at the DM root. Main routes all prompts through the single _send_prompt prompt-op choke point, so strip the synthetic DM thread anchor there via _strip_synthetic_dm_thread — preserving real threads (distinct thread_id), tenant scope (scope_id/slack_team_id), and non-DM/non-Slack chats. Preserves main's hp1 prompt-codec; no competing ap:/cl: encoding. --- .../relay/test_relay_slack_prompt_dm_root.py | 194 ++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 tests/gateway/relay/test_relay_slack_prompt_dm_root.py diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py new file mode 100644 index 00000000000..edee3851866 --- /dev/null +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -0,0 +1,194 @@ +"""Slack relay: interactive prompts (approval / clarify) must post FLAT at the DM root. + +Reported symptom (live): an approval / clarify Block Kit card raised mid-turn in +a Slack DM was posted THREADED under the triggering message instead of at the DM +root ("the approval block was put in a thread and did not follow the setting"). + +Root cause: a clarify/approval prompt is emitted in reply to the triggering +inbound event, so the metadata handed to ``_send_prompt`` carries that event's +thread context — run.py's ``_thread_metadata_for_source`` stamps +``metadata["thread_id"]`` (for a Slack DM, the triggering message's own ts, used +only as a session-keying fallback), plus ``metadata["message_id"]`` = that same +ts. Forwarding ``thread_id`` makes the connector's slackRestSender thread the +prompt card UNDER the user's message. Native Slack Hermes suppresses this +synthetic DM thread anchor (``SlackAdapter._resolve_thread_ts``); the relay lane +had no such disambiguation. + +These are behaviour-contract tests: they assert how the outbound ``prompt`` frame +relates to the chat type + inherited thread metadata (the invariant the connector +depends on), not a snapshot. They drive the REAL ``RelayAdapter`` + +``StubConnector`` end to end. +""" + +from __future__ import annotations + +import pytest + +from gateway.config import Platform, PlatformConfig +from gateway.platforms.base import MessageEvent, MessageType +from gateway.relay.adapter import RelayAdapter +from gateway.relay.descriptor import CONTRACT_VERSION, CapabilityDescriptor +from gateway.session import SessionSource + +from tests.gateway.relay.stub_connector import StubConnector + +FULL_OPS = ("send", "edit", "typing", "get_chat_info", "send_media", "prompt", "react") + + +def _slack_desc(**kw) -> CapabilityDescriptor: + base = dict( + contract_version=CONTRACT_VERSION, + platform="slack", + label="Slack", + max_message_length=4000, + supports_draft_streaming=False, + supports_edit=True, + supports_threads=True, + markdown_dialect="mrkdwn", + len_unit="chars", + supported_ops=FULL_OPS, + ) + base.update(kw) + return CapabilityDescriptor(**base) + + +def _wire( + chat_id: str, + chat_type: str, + *, + user_id="U1", + scope_id=None, + platform=Platform.SLACK, +): + """A RelayAdapter fronting Slack, with inbound scope + chat_type captured.""" + stub = StubConnector(_slack_desc()) + adapter = RelayAdapter(PlatformConfig(), _slack_desc(), transport=stub) + src = SessionSource( + platform=platform, + chat_id=chat_id, + chat_type=chat_type, + user_id=user_id, + scope_id=scope_id, + ) + adapter._capture_scope( + MessageEvent(text="hi", source=src, message_type=MessageType.TEXT) + ) + return adapter, stub + + +def _last_prompt(stub) -> dict: + prompts = [f for f in stub.sent if f["op"] == "prompt"] + assert prompts, "expected a prompt op on the wire" + return prompts[-1] + + +# --------------------------------------------------------------------------- +# DM-root: the synthetic self-anchor is stripped +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_exec_approval_posts_flat_at_dm_root(): + """A Slack DM approval prompt must NOT inherit the triggering message's + synthetic thread_id — it posts flat at the DM root, matching native.""" + adapter, stub = _wire("D1", "dm", scope_id="T1") + # run.py hands the prompt the triggering message's thread context: for a DM + # with no real thread, thread_id == message_id (the synthetic self-anchor). + md = { + "thread_id": "1700000000.000100", + "message_id": "1700000000.000100", + "scope_id": "T1", + } + result = await adapter.send_exec_approval( + "D1", "rm -rf /tmp/x", "sess:1", description="deletes files", metadata=md + ) + assert result.success is True + frame = _last_prompt(stub) + meta = frame["metadata"] or {} + # The inherited synthetic thread anchor is dropped so it posts at the DM root. + assert "thread_id" not in meta, ( + "approval prompt must NOT inherit the triggering message thread_id" + ) + assert "thread_ts" not in meta + # reply_to on the outbound action stays unset — a root-level post. + assert frame["reply_to"] is None + # Tenant scope is preserved untouched (egress routing must not break). + assert meta.get("scope_id") == "T1" + # The caller's original metadata dict was not mutated in place. + assert md.get("thread_id") == "1700000000.000100" + + +@pytest.mark.asyncio +async def test_clarify_posts_flat_at_dm_root(): + """A Slack DM clarify prompt (with choices) also posts flat at the DM root.""" + adapter, stub = _wire("D1", "dm", scope_id="T1") + md = { + "thread_id": "1700000000.000200", + "message_id": "1700000000.000200", + "scope_id": "T1", + } + result = await adapter.send_clarify( + "D1", "Which env?", ["prod", "staging"], "cl-1", "sess:1", metadata=md + ) + assert result.success is True + frame = _last_prompt(stub) + meta = frame["metadata"] or {} + assert "thread_id" not in meta + assert "thread_ts" not in meta + assert frame["reply_to"] is None + assert meta.get("scope_id") == "T1" + + +@pytest.mark.asyncio +async def test_slash_confirm_posts_flat_at_dm_root(): + """The DM-root rule covers every prompt surface (single _send_prompt choke).""" + adapter, stub = _wire("D1", "dm") + md = {"thread_id": "1700000000.000300", "message_id": "1700000000.000300"} + await adapter.send_slash_confirm( + "D1", "Reload MCP", "invalidates cache", "s", "cf-1", metadata=md + ) + frame = _last_prompt(stub) + assert "thread_id" not in (frame["metadata"] or {}) + + +# --------------------------------------------------------------------------- +# Regression guards: a REAL thread and non-DM / non-Slack chats are untouched +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_exec_approval_in_real_thread_keeps_thread_id(): + """A DM prompt raised inside a REAL thread (thread_id distinct from the + triggering message ts) must STAY in that thread — only the synthetic + self-anchor is stripped.""" + adapter, stub = _wire("D1", "dm", scope_id="T1") + md = { + "thread_id": "1699000000.999000", + "message_id": "1700000000.000100", + "scope_id": "T1", + } + await adapter.send_exec_approval("D1", "cmd", "s", metadata=md) + frame = _last_prompt(stub) + assert frame["metadata"]["thread_id"] == "1699000000.999000" + + +@pytest.mark.asyncio +async def test_channel_approval_keeps_thread_id(): + """A Slack CHANNEL prompt keeps its thread_id (autoThread / real thread); + the DM-only guard must not touch a non-DM chat.""" + adapter, stub = _wire("C1", "channel", scope_id="T1") + md = { + "thread_id": "1700000000.000400", + "message_id": "1700000000.000400", + "scope_id": "T1", + } + await adapter.send_exec_approval("C1", "cmd", "s", metadata=md) + frame = _last_prompt(stub) + assert frame["metadata"]["thread_id"] == "1700000000.000400" + + +@pytest.mark.asyncio +async def test_non_slack_dm_approval_keeps_thread_id(): + """The disambiguation is Slack-scoped: a non-Slack relay DM keeps thread_id + (its connector owns its own threading semantics).""" + adapter, stub = _wire("dc1", "dm", platform=Platform.DISCORD) + md = {"thread_id": "9000", "message_id": "9000"} + await adapter.send_exec_approval("dc1", "cmd", "s", metadata=md) + frame = _last_prompt(stub) + assert frame["metadata"]["thread_id"] == "9000" From e286658377ed63f17582be59bdc081811ed63b3d Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Sun, 26 Jul 2026 20:59:52 +0000 Subject: [PATCH 03/20] fix(scripts): encode tool_search_livetest2 output as utf-8 (Windows footgun) check-windows-footguns (blocking CI) flagged a bare Path.write_text() without encoding= at scripts/tool_search_livetest2.py:190, which uses the platform locale encoding on Windows. Pin utf-8. Pre-existing on main; unblocks the required-checks gate for this PR. --- scripts/tool_search_livetest2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tool_search_livetest2.py b/scripts/tool_search_livetest2.py index b81f91f91af..522b141685c 100644 --- a/scripts/tool_search_livetest2.py +++ b/scripts/tool_search_livetest2.py @@ -187,7 +187,7 @@ def run_one(scenario: Dict[str, Any], mode: str, rep: int, out_dir: Path) -> Dic "final_response": base._redact_secrets(final_response)[:500], } out_path = out_dir / f"{scenario['id']}__{'enabled' if enabled else 'disabled'}__rep{rep}.json" - out_path.write_text(json.dumps(rec, indent=1)) + out_path.write_text(json.dumps(rec, indent=1), encoding="utf-8") shutil.rmtree(Path(os.environ["HERMES_HOME"]).parent, ignore_errors=True) return rec From 42e4f70eefdd9651b50b86f3c66542ca28d69ff0 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Sun, 26 Jul 2026 21:32:22 +0000 Subject: [PATCH 04/20] fix(relay): native-parity Slack approval button styles + labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack Block Kit buttons only support style primary (green) / danger (red) / default (white). The relay approval + slash-confirm prompts emitted an invalid style 'success' (Slack silently drops it → white/stroke button) and baked emoji into the labels (non-native). Native Slack Hermes uses plain labels with primary/danger. Map to valid styles (once→primary, deny/cancel→danger, session/always→default) and drop the emoji from labels. The connector already compensates success→primary, but emitting valid values at the source is correct and removes the fragile dependency on that compensation. --- gateway/relay/adapter.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index b544748208a..e50b9771ed5 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -1399,16 +1399,15 @@ class RelayAdapter(BasePlatformAdapter): button→text fallback takes over (same contract as a native adapter's failed button send). """ - options: list = [{"id": "once", "label": "✅ Allow Once", "style": "success"}] + options: list = [{"id": "once", "label": "Allow Once", "style": "primary"}] if not smart_denied and allow_session: - options.append({"id": "session", "label": "✅ Session", "style": "primary"}) + options.append({"id": "session", "label": "Allow Session"}) if allow_permanent: options.append({ "id": "always", - "label": "✅ Always", - "style": "primary", + "label": "Always Allow", }) - options.append({"id": "deny", "label": "❌ Deny", "style": "danger"}) + options.append({"id": "deny", "label": "Deny", "style": "danger"}) cmd_preview = command if len(command) <= 1500 else command[:1500] + "..." text = ( @@ -1455,9 +1454,9 @@ class RelayAdapter(BasePlatformAdapter): gateway's text-intercept flow when the prompt lane is unavailable. """ options = [ - {"id": "once", "label": "✅ Approve Once", "style": "success"}, - {"id": "always", "label": "🔒 Always Approve", "style": "primary"}, - {"id": "cancel", "label": "❌ Cancel", "style": "danger"}, + {"id": "once", "label": "Approve Once", "style": "primary"}, + {"id": "always", "label": "Always Approve"}, + {"id": "cancel", "label": "Cancel", "style": "danger"}, ] text = f"**{title}**\n\n{message}" if title else message prompt_id = self._mint_prompt( From 95103db64552f05da214eff7d97191d8530534f4 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 13:50:13 +0000 Subject: [PATCH 05/20] =?UTF-8?q?fix(relay):=20prompts=20trust=20the=20run?= =?UTF-8?q?.py=20thread=20stamp=20=E2=80=94=20no=20self-anchor=20re-deriva?= =?UTF-8?q?tion=20(QA-5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The threading mode (flat vs thread-per-message) is decided once, in run.py's _resolve_progress_thread_id (reply_in_thread knob), and encoded in the metadata stamp: flat => no thread_id, threaded => thread_id for the turn (first turn: == message_id, the synthetic root IS the thread). _strip_synthetic_dm_thread re-derived the mode with an unconditional thread_id == message_id strip, exiling approval/clarify cards (and their resolved-state swaps) to the DM root while progress bubbles honoured the thread (2026-07-27 mixed-placement report). Trust the stamp instead; flat mode is unaffected because flat metadata never carries an anchor. --- gateway/relay/adapter.py | 28 +++-- .../relay/test_relay_slack_prompt_dm_root.py | 104 ++++++++++-------- 2 files changed, 77 insertions(+), 55 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index e50b9771ed5..57ad659d6d5 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -1301,17 +1301,23 @@ class RelayAdapter(BasePlatformAdapter): thread_id = metadata.get("thread_id") if not thread_id: return metadata - # A real thread carries a thread_id distinct from the triggering message - # ts (run.py stamps that ts as metadata["message_id"] on Slack). Only the - # synthetic self-anchor (thread_id == that ts, or no distinguishing anchor - # present) is stripped; a genuine thread is honoured. - anchor = metadata.get("message_id") - if anchor is not None and str(thread_id) != str(anchor): - return metadata - cleaned = dict(metadata) - cleaned.pop("thread_id", None) - cleaned.pop("thread_ts", None) - return cleaned + # Trust the run.py stamp (QA-5). The threading MODE is decided in ONE + # place — run.py's _resolve_progress_thread_id, which reads + # platforms.slack.extra.reply_in_thread: + # * flat mode (reply_in_thread=false): the synthetic self-anchor is + # suppressed THERE, so prompt metadata arrives with NO thread_id and + # this helper is a no-op — the card posts flat at the DM root; + # * thread-per-message mode (default): metadata.thread_id is stamped + # for the whole turn, and on the FIRST turn it legitimately equals + # the triggering message's ts (the synthetic root IS the thread). + # The previous unconditional thread_id == message_id strip re-derived + # the mode here and got it wrong for thread-per-message: the approval + # card (and its resolved-state swap) was exiled to the DM root while + # progress bubbles honoured the thread (2026-07-27 mixed-placement + # screenshot). Mirror native SlackAdapter._resolve_thread_ts, which + # only performs the self-anchor strip when reply_in_thread=false — a + # state this lane never sees with an anchor present, per the above. + return metadata async def _send_prompt( self, diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index edee3851866..f08ba30dec8 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -1,21 +1,24 @@ -"""Slack relay: interactive prompts (approval / clarify) must post FLAT at the DM root. +"""Slack relay: interactive prompts follow the turn's thread stamp (QA-5). -Reported symptom (live): an approval / clarify Block Kit card raised mid-turn in -a Slack DM was posted THREADED under the triggering message instead of at the DM -root ("the approval block was put in a thread and did not follow the setting"). +The threading MODE (flat DM vs thread-per-message) is decided in exactly ONE +place: run.py's ``_resolve_progress_thread_id``, which reads +``platforms.slack.extra.reply_in_thread`` and encodes the verdict into the +outbound ``metadata`` stamp: -Root cause: a clarify/approval prompt is emitted in reply to the triggering -inbound event, so the metadata handed to ``_send_prompt`` carries that event's -thread context — run.py's ``_thread_metadata_for_source`` stamps -``metadata["thread_id"]`` (for a Slack DM, the triggering message's own ts, used -only as a session-keying fallback), plus ``metadata["message_id"]`` = that same -ts. Forwarding ``thread_id`` makes the connector's slackRestSender thread the -prompt card UNDER the user's message. Native Slack Hermes suppresses this -synthetic DM thread anchor (``SlackAdapter._resolve_thread_ts``); the relay lane -had no such disambiguation. + * flat mode -> the synthetic self-anchor is suppressed in run.py, so prompt + metadata arrives with NO ``thread_id`` and the card posts at the DM root; + * thread-per-message (default) -> ``metadata.thread_id`` is stamped for the + whole turn; on the FIRST turn it legitimately equals the triggering + message's ts (the synthetic root IS the thread). -These are behaviour-contract tests: they assert how the outbound ``prompt`` frame -relates to the chat type + inherited thread metadata (the invariant the connector +The prompt lane must TRUST that stamp, like ``_resolve_reply_to_for_send`` +does. Re-deriving the mode here (the old unconditional +``thread_id == message_id`` strip) exiled the approval card and its +resolved-state swap to the DM root while progress bubbles honoured the thread +(the 2026-07-27 mixed-placement report). + +These are behaviour-contract tests: they assert how the outbound ``prompt`` +frame relates to the inherited thread metadata (the invariant the connector depends on), not a snapshot. They drive the REAL ``RelayAdapter`` + ``StubConnector`` end to end. """ @@ -83,15 +86,39 @@ def _last_prompt(stub) -> dict: # --------------------------------------------------------------------------- -# DM-root: the synthetic self-anchor is stripped +# Flat mode: run.py stamps NO thread_id -> the card posts at the DM root. # --------------------------------------------------------------------------- @pytest.mark.asyncio -async def test_exec_approval_posts_flat_at_dm_root(): - """A Slack DM approval prompt must NOT inherit the triggering message's - synthetic thread_id — it posts flat at the DM root, matching native.""" +async def test_exec_approval_flat_mode_posts_at_dm_root(): + """Flat-DM turn (reply_in_thread=false): run.py suppressed the synthetic + anchor upstream, so prompt metadata has no thread_id and none appears on + the wire — the card posts at the DM root.""" + adapter, stub = _wire("D1", "dm", scope_id="T1") + md = {"message_id": "1700000000.000100", "scope_id": "T1"} + result = await adapter.send_exec_approval( + "D1", "rm -rf /tmp/x", "sess:1", description="deletes files", metadata=md + ) + assert result.success is True + frame = _last_prompt(stub) + meta = frame["metadata"] or {} + assert "thread_id" not in meta + assert "thread_ts" not in meta + # reply_to on the outbound action stays unset — a root-level post. + assert frame["reply_to"] is None + # Tenant scope is preserved untouched (egress routing must not break). + assert meta.get("scope_id") == "T1" + + +# --------------------------------------------------------------------------- +# Thread-per-message mode: the first-turn self-anchor (thread_id == message_id) +# IS the thread root — the prompt must stay in the thread (QA-5 regression). +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_exec_approval_first_turn_self_anchor_stays_in_thread(): + """Thread-per-message first turn: run.py stamps thread_id = the triggering + message's own ts. The approval card must post INTO that thread — stripping + it exiled the card to the home channel (2026-07-27 report).""" adapter, stub = _wire("D1", "dm", scope_id="T1") - # run.py hands the prompt the triggering message's thread context: for a DM - # with no real thread, thread_id == message_id (the synthetic self-anchor). md = { "thread_id": "1700000000.000100", "message_id": "1700000000.000100", @@ -103,22 +130,14 @@ async def test_exec_approval_posts_flat_at_dm_root(): assert result.success is True frame = _last_prompt(stub) meta = frame["metadata"] or {} - # The inherited synthetic thread anchor is dropped so it posts at the DM root. - assert "thread_id" not in meta, ( - "approval prompt must NOT inherit the triggering message thread_id" + assert meta.get("thread_id") == "1700000000.000100", ( + "first-turn self-anchor is the thread root; the prompt must honour it" ) - assert "thread_ts" not in meta - # reply_to on the outbound action stays unset — a root-level post. - assert frame["reply_to"] is None - # Tenant scope is preserved untouched (egress routing must not break). assert meta.get("scope_id") == "T1" - # The caller's original metadata dict was not mutated in place. - assert md.get("thread_id") == "1700000000.000100" @pytest.mark.asyncio -async def test_clarify_posts_flat_at_dm_root(): - """A Slack DM clarify prompt (with choices) also posts flat at the DM root.""" +async def test_clarify_first_turn_self_anchor_stays_in_thread(): adapter, stub = _wire("D1", "dm", scope_id="T1") md = { "thread_id": "1700000000.000200", @@ -131,22 +150,21 @@ async def test_clarify_posts_flat_at_dm_root(): assert result.success is True frame = _last_prompt(stub) meta = frame["metadata"] or {} - assert "thread_id" not in meta - assert "thread_ts" not in meta - assert frame["reply_to"] is None + assert meta.get("thread_id") == "1700000000.000200" assert meta.get("scope_id") == "T1" @pytest.mark.asyncio -async def test_slash_confirm_posts_flat_at_dm_root(): - """The DM-root rule covers every prompt surface (single _send_prompt choke).""" +async def test_slash_confirm_first_turn_self_anchor_stays_in_thread(): + """The stamp-trusting rule covers every prompt surface (single + _send_prompt choke point).""" adapter, stub = _wire("D1", "dm") md = {"thread_id": "1700000000.000300", "message_id": "1700000000.000300"} await adapter.send_slash_confirm( "D1", "Reload MCP", "invalidates cache", "s", "cf-1", metadata=md ) frame = _last_prompt(stub) - assert "thread_id" not in (frame["metadata"] or {}) + assert (frame["metadata"] or {}).get("thread_id") == "1700000000.000300" # --------------------------------------------------------------------------- @@ -155,8 +173,7 @@ async def test_slash_confirm_posts_flat_at_dm_root(): @pytest.mark.asyncio async def test_exec_approval_in_real_thread_keeps_thread_id(): """A DM prompt raised inside a REAL thread (thread_id distinct from the - triggering message ts) must STAY in that thread — only the synthetic - self-anchor is stripped.""" + triggering message ts) stays in that thread.""" adapter, stub = _wire("D1", "dm", scope_id="T1") md = { "thread_id": "1699000000.999000", @@ -170,8 +187,7 @@ async def test_exec_approval_in_real_thread_keeps_thread_id(): @pytest.mark.asyncio async def test_channel_approval_keeps_thread_id(): - """A Slack CHANNEL prompt keeps its thread_id (autoThread / real thread); - the DM-only guard must not touch a non-DM chat.""" + """A Slack CHANNEL prompt keeps its thread_id (autoThread / real thread).""" adapter, stub = _wire("C1", "channel", scope_id="T1") md = { "thread_id": "1700000000.000400", @@ -185,8 +201,8 @@ async def test_channel_approval_keeps_thread_id(): @pytest.mark.asyncio async def test_non_slack_dm_approval_keeps_thread_id(): - """The disambiguation is Slack-scoped: a non-Slack relay DM keeps thread_id - (its connector owns its own threading semantics).""" + """A non-Slack relay DM keeps thread_id (its connector owns its own + threading semantics).""" adapter, stub = _wire("dc1", "dm", platform=Platform.DISCORD) md = {"thread_id": "9000", "message_id": "9000"} await adapter.send_exec_approval("dc1", "cmd", "s", metadata=md) From be9de31967f1b11c1d37ad7a8e337c48bbb3745d Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 14:37:10 +0000 Subject: [PATCH 06/20] =?UTF-8?q?fix(relay):=20final=20DM=20reply=20honors?= =?UTF-8?q?=20thread-per-message=20mode=20=E2=80=94=20gate=20the=20reply?= =?UTF-8?q?=5Fto=20strip=20on=20reply=5Fin=5Fthread=20(QA-6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _resolve_reply_to_for_send dropped the triggering-ts reply_to on every Slack DM with no metadata thread_id. But the final-reply lane (platforms/ base.py) builds metadata from source.thread_id only — None for a top-level DM — so in thread-per-message mode that reply_to is the final reply's ONLY threading signal, and stripping it exiled the final message to the DM root while progress bubbles stayed threaded (sibling of the QA-5 prompt bug). Mirror native _resolve_thread_ts: suppress the synthetic anchor only when platforms.slack.extra.reply_in_thread=false. Flat mode behavior unchanged; real threads and channels unchanged. --- gateway/relay/adapter.py | 20 ++++++- .../relay/test_relay_slack_dm_streaming.py | 53 ++++++++++++++++--- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 57ad659d6d5..7bf687cb49f 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -814,7 +814,25 @@ class RelayAdapter(BasePlatformAdapter): if md.get("thread_id") or md.get("thread_ts"): # A real thread was resolved by run.py — honour it. return reply_to - # Synthetic DM self-anchor: post flat at the DM root (native parity). + # Mode gate (native _resolve_thread_ts parity). The final-reply lane + # (gateway/platforms/base.py) builds metadata from source.thread_id + # ONLY — for a top-level DM that is None, so in thread-per-message + # mode the triggering-ts reply_to here is the final reply's ONLY + # threading signal (run.py's synthetic root feeds just the + # progress/status lane). Dropping it unconditionally exiled the final + # message to the DM root while progress stayed threaded (2026-07-27 + # report, sibling of the QA-5 prompt bug). Native SlackAdapter only + # suppresses the anchor when reply_in_thread=false; mirror that. + try: + reply_in_thread = bool( + (self.config.extra or {}).get("reply_in_thread", True) + ) + except Exception: # noqa: BLE001 - config shape is adapter-owned + reply_in_thread = True + if reply_in_thread: + # Thread-per-message: the triggering ts is the thread anchor. + return reply_to + # Flat mode: synthetic DM self-anchor — post flat at the DM root. return None async def edit_message( diff --git a/tests/gateway/relay/test_relay_slack_dm_streaming.py b/tests/gateway/relay/test_relay_slack_dm_streaming.py index 008f6c608c0..35b3d5f989c 100644 --- a/tests/gateway/relay/test_relay_slack_dm_streaming.py +++ b/tests/gateway/relay/test_relay_slack_dm_streaming.py @@ -73,18 +73,33 @@ def _wire(chat_id: str, chat_type: str, *, user_id="U1", scope_id=None): # The pure disambiguation contract (RelayAdapter.send) # --------------------------------------------------------------------------- @pytest.mark.asyncio -async def test_slack_dm_reply_drops_synthetic_thread_anchor(): - """A Slack DM reply with no real thread posts FLAT: reply_to is dropped so - the connector cannot thread it under the triggering message.""" +async def test_slack_dm_reply_keeps_anchor_in_thread_per_message_mode(): + """Default mode (reply_in_thread=True, thread-per-message): the triggering + ts reply_to is the final reply's ONLY threading signal (base.py builds + metadata from source.thread_id, which is None for a top-level DM) — it + must be KEPT so the final message lands in the per-message thread with + the progress bubbles (2026-07-27 mixed-placement report).""" adapter, stub = _wire("D1", "dm") await adapter.send("D1", "the answer", reply_to="1700.0001") assert len(stub.sent) == 1 frame = stub.sent[0] assert frame["op"] == "send" - # The synthetic self-anchor is suppressed on BOTH surfaces. + assert frame["reply_to"] == "1700.0001", ( + "thread-per-message: the triggering ts anchors the final reply" + ) + + +@pytest.mark.asyncio +async def test_slack_dm_reply_drops_synthetic_anchor_in_flat_mode(): + """Flat mode (reply_in_thread=False): the synthetic self-anchor is dropped + so the reply posts flat at the DM root (native _resolve_thread_ts parity) + and no synthetic thread is invented (#18859).""" + adapter, stub = _wire("D1", "dm") + adapter.config.extra = {"reply_in_thread": False} + await adapter.send("D1", "the answer", reply_to="1700.0001") + frame = stub.sent[0] assert frame["reply_to"] is None assert "thread_id" not in (frame["metadata"] or {}) - # And no synthetic thread_id was invented (the #18859 landmine). assert "thread_ts" not in (frame["metadata"] or {}) @@ -165,8 +180,13 @@ async def test_slack_dm_stream_consumer_edits_own_ts_not_flat(): The connector returns a real message_id for the flat first send, so edit support must stay on and at least one edit op must be emitted (progressive - streaming), identical to a thread. No synthetic thread is created.""" + streaming), identical to a thread. No synthetic thread is created. + + Runs in EXPLICIT flat mode (reply_in_thread=False) — that is the mode this + contract belongs to; the default thread-per-message path is covered by + test_slack_dm_stream_consumer_threads_in_thread_per_message_mode.""" adapter, stub = _wire("D1", "dm") + adapter.config.extra = {"reply_in_thread": False} consumer = await _drive_stream( adapter, "D1", @@ -218,3 +238,24 @@ async def test_slack_thread_stream_consumer_still_threads_and_streams(): # Thread preserved: the real thread_id rides along and reply_to is kept. assert first_send["metadata"]["thread_id"] == "1699.9000" assert first_send["reply_to"] == "1700.0002" + + +@pytest.mark.asyncio +async def test_slack_dm_stream_consumer_threads_in_thread_per_message_mode(): + """Default mode: the DM stream's first send keeps the triggering-ts anchor + so the streamed final reply lands in the per-message thread; edits still + target the reply's own ts.""" + adapter, stub = _wire("D1", "dm") + consumer = await _drive_stream( + adapter, + "D1", + metadata=None, + initial_reply_to_id="1700.0001", + chat_type="dm", + ) + first_send = stub.sent[0] + assert first_send["op"] == "send" + assert first_send["reply_to"] == "1700.0001" + assert consumer.message_id and consumer.message_id != "__no_edit__" + edit_ids = {f["message_id"] for f in stub.sent if f["op"] == "edit"} + assert edit_ids <= {stub.next_send_result["message_id"]} From b493bf63c7e327bed92a47937edd87855d7f52f7 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 14:04:46 +0000 Subject: [PATCH 07/20] =?UTF-8?q?feat(relay):=20rich=20Slack=20status-line?= =?UTF-8?q?=20parity=20=E2=80=94=20advertise=20supports=5Fstatus=5Ftext,?= =?UTF-8?q?=20carry=20live=20per-tool=20phrase=20on=20typing=20frames=20(Q?= =?UTF-8?q?A-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Native Slack shows dynamic assistant-status text ('Finding answers…', 'is running pytest…') because SlackAdapter sets supports_status_text=True and renders the set_status_text() phrase in send_typing. The relay lane advertised nothing, so run.py's live-status lane never fed it phrases and the connector fell back to the static default. - supports_status_text: descriptor-gated property (Slack only; other fronted platforms keep textless bubbles) - send_typing: carry the stashed phrase as the typing op's content; omit when unset (empty string is Slack's explicit clear, reserved for stop_typing). Connector already renders content via assistant.threads.setStatus (#154). --- gateway/relay/adapter.py | 40 ++++++++++++++++--- .../relay/test_relay_slack_prompt_dm_root.py | 38 ++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 7bf687cb49f..fff21bb6a94 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -133,6 +133,23 @@ class RelayAdapter(BasePlatformAdapter): def message_len_fn(self) -> Callable[[str], int]: return _LEN_FNS.get(self.descriptor.len_unit, len) + @property + def supports_status_text(self) -> bool: # type: ignore[override] + """Whether the fronted platform renders a TEXT status line. + + Native parity (QA-1 rich status): Slack's typing surface is the + assistant status line ("Finding answers…" next to the bot name), a + text-rendering indicator. When the relay fronts Slack, advertise it so + run.py's live-status lane feeds per-tool phrases via + ``set_status_text()`` — exactly the wiring the native SlackAdapter + gets (``supports_status_text = True``). Other fronted platforms keep + textless typing bubbles and must NOT receive phrase traffic. + + Property (not class attr) because ONE RelayAdapter class fronts many + platforms; the answer depends on the handshaked descriptor. + """ + return self.descriptor.platform == Platform.SLACK.value + def supports_draft_streaming( self, chat_type: Optional[str] = None, @@ -892,13 +909,26 @@ class RelayAdapter(BasePlatformAdapter): """ if self._transport is None: return + # Rich status parity (QA-1): run.py's live-status lane stashes the + # current per-tool phrase via set_status_text() (base class store). + # Carry it as the typing frame's content so the connector's Slack + # sender renders it on assistant.threads.setStatus — the same phrase + # the native adapter shows ("is running pytest…", "Finding answers…"). + # Absent (None/empty) => omit content; the connector falls back to its + # default "is typing…" heartbeat, preserving pre-phrase behaviour on + # every platform. Never send empty-string content here: on Slack that + # is the explicit CLEAR request reserved for stop_typing. + frame: Dict[str, Any] = { + "op": "typing", + "chat_id": chat_id, + "metadata": self._with_scope(chat_id, metadata), + } + phrase = getattr(self, "_status_text", {}).get(str(chat_id)) + if phrase: + frame["content"] = str(phrase) try: await self._transport.send_outbound( - { - "op": "typing", - "chat_id": chat_id, - "metadata": self._with_scope(chat_id, metadata), - }, + frame, platform=self._platform_by_chat.get(str(chat_id)), ) except Exception: # noqa: BLE001 - typing is cosmetic, never breaks a turn diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index f08ba30dec8..5e48e972b8d 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -208,3 +208,41 @@ async def test_non_slack_dm_approval_keeps_thread_id(): await adapter.send_exec_approval("dc1", "cmd", "s", metadata=md) frame = _last_prompt(stub) assert frame["metadata"]["thread_id"] == "9000" + + +# --------------------------------------------------------------------------- +# QA-1 rich status: the relay advertises Slack's text status line and carries +# the live per-tool phrase on the typing frame (native set_status_text parity). +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_slack_relay_advertises_status_text(): + adapter, _stub = _wire("D1", "dm") + assert adapter.supports_status_text is True + + +@pytest.mark.asyncio +async def test_non_slack_relay_does_not_advertise_status_text(): + stub = StubConnector(_slack_desc(platform="discord")) + adapter = RelayAdapter( + PlatformConfig(), _slack_desc(platform="discord"), transport=stub + ) + assert adapter.supports_status_text is False + + +@pytest.mark.asyncio +async def test_typing_carries_live_status_phrase(): + """set_status_text() -> the next typing frame carries the phrase as + content; clearing it (None) reverts to a content-less heartbeat frame + (never an empty string, which is Slack's explicit clear).""" + adapter, stub = _wire("D1", "dm", scope_id="T1") + adapter.set_status_text("D1", "is running pytest…") + await adapter.send_typing("D1", metadata={"scope_id": "T1"}) + typing = [f for f in stub.sent if f["op"] == "typing"] + assert typing and typing[-1].get("content") == "is running pytest…" + + adapter.set_status_text("D1", None) + await adapter.send_typing("D1", metadata={"scope_id": "T1"}) + typing = [f for f in stub.sent if f["op"] == "typing"] + assert "content" not in typing[-1], ( + "cleared phrase must omit content (empty string means CLEAR on Slack)" + ) From 467534b43ed87e55861d0ef87ebd7822f313bc3d Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 15:03:51 +0000 Subject: [PATCH 08/20] =?UTF-8?q?fix(relay):=20typing/status=20targets=20t?= =?UTF-8?q?he=20per-message=20thread=20=E2=80=94=20synthesize=20the=20anch?= =?UTF-8?q?or=20from=20the=20inbound=20ts=20(QA-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack's thinking-status line (thread replies footer, plain chat:write — no assistant scopes needed) is thread-only: the connector's typing case no-ops without thread_ts. The typing lane's metadata has no anchor for a top-level DM (base.py builds from source.thread_id = None), so every status heartbeat was silently dropped — the trace showed typing frames with meta_keys=['user_id'] only. Cache the triggering message ts per chat on inbound (_capture_scope) and synthesize metadata.thread_id on send_typing/stop_typing in thread-per-message mode, mirroring native send_typing's _resolve_thread_ts(metadata.message_id). Flat mode unchanged (#18859); real-thread metadata wins over the cache; the clear frame targets the same synthesized thread so the status never sticks. --- gateway/relay/adapter.py | 66 ++++++++++++++++++- .../relay/test_relay_slack_prompt_dm_root.py | 60 +++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index fff21bb6a94..8b125a22adc 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -82,6 +82,10 @@ class RelayAdapter(BasePlatformAdapter): # synthetic reply_to in _resolve_thread_ts; the relay lane needs the same # disambiguation, and it needs the chat_type to know a chat is a DM. self._chat_type_by_chat: Dict[str, str] = {} + # chat_id -> last triggering message ts (Slack). The typing/status + # lane's synthetic thread anchor in thread-per-message mode (QA-1); + # see _capture_scope and send_typing. + self._last_inbound_ts_by_chat: Dict[str, str] = {} # chat_id -> the UNDERLYING platform (e.g. "discord", "telegram") this # chat belongs to (Phase 1.5 multi-platform-per-agent). One relay adapter # fronts N platforms on one WS; an outbound reply must egress through the @@ -378,6 +382,19 @@ class RelayAdapter(BasePlatformAdapter): chat_type = getattr(src, "chat_type", None) if chat_type: self._chat_type_by_chat[str(chat)] = str(chat_type) + # Triggering message ts (QA-1): the typing/status lane's metadata + # (base.py _thread_metadata_for_source) carries NO thread anchor + # for a top-level DM, but in thread-per-message mode the status + # must target the per-message thread (its root = this ts). Cache + # it per chat so send_typing can synthesize the anchor, mirroring + # native send_typing's _resolve_thread_ts(metadata.message_id). + # NOTE: message_id lives on the EVENT (MessageEvent), not the + # source — fall back to source for defensive coverage. + message_id = getattr(event, "message_id", None) or getattr( + src, "message_id", None + ) + if message_id: + self._last_inbound_ts_by_chat[str(chat)] = str(message_id) except Exception: # noqa: BLE001 - scope tracking must never break inbound pass @@ -909,6 +926,33 @@ class RelayAdapter(BasePlatformAdapter): """ if self._transport is None: return + # Thread anchor for the status surface (QA-1). Slack's status line + # ("is thinking…" in the thread's replies footer — works with plain + # chat:write, confirmed on native no-assistant bots) is THREAD-only: + # the connector's typing case no-ops without a thread_ts. But the + # typing lane's metadata (base.py _thread_metadata_for_source) has no + # anchor for a top-level DM — source.thread_id is None — so every + # heartbeat was silently dropped. In thread-per-message mode the + # turn's thread root IS the triggering message ts (run.py's synthetic + # root); synthesize it here from the per-chat inbound cache, exactly + # like native send_typing resolves thread_ts from metadata.message_id. + # Flat mode (reply_in_thread=false) keeps the no-anchor no-op: there + # is no thread and must not be one (#18859). + md = dict(metadata or {}) + if ( + not (md.get("thread_id") or md.get("thread_ts")) + and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value + and self._chat_type_by_chat.get(str(chat_id)) == "dm" + ): + try: + reply_in_thread = bool( + (self.config.extra or {}).get("reply_in_thread", True) + ) + except Exception: # noqa: BLE001 - config shape is adapter-owned + reply_in_thread = True + anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) + if reply_in_thread and anchor: + md["thread_id"] = anchor # Rich status parity (QA-1): run.py's live-status lane stashes the # current per-tool phrase via set_status_text() (base class store). # Carry it as the typing frame's content so the connector's Slack @@ -921,7 +965,7 @@ class RelayAdapter(BasePlatformAdapter): frame: Dict[str, Any] = { "op": "typing", "chat_id": chat_id, - "metadata": self._with_scope(chat_id, metadata), + "metadata": self._with_scope(chat_id, md), } phrase = getattr(self, "_status_text", {}).get(str(chat_id)) if phrase: @@ -957,13 +1001,31 @@ class RelayAdapter(BasePlatformAdapter): platform = self._platform_by_chat.get(str(chat_id)) if platform != Platform.SLACK.value: return + # Clear must target the SAME thread the heartbeat set (QA-1): apply + # the identical synthetic-anchor rule as send_typing, or the clear + # frame no-ops threadless and the status line sticks until Slack's + # own timeout. + md = dict(metadata or {}) + if ( + not (md.get("thread_id") or md.get("thread_ts")) + and self._chat_type_by_chat.get(str(chat_id)) == "dm" + ): + try: + reply_in_thread = bool( + (self.config.extra or {}).get("reply_in_thread", True) + ) + except Exception: # noqa: BLE001 - config shape is adapter-owned + reply_in_thread = True + anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) + if reply_in_thread and anchor: + md["thread_id"] = anchor try: await self._transport.send_outbound( { "op": "typing", "chat_id": chat_id, "content": "", - "metadata": self._with_scope(chat_id, metadata), + "metadata": self._with_scope(chat_id, md), }, platform=platform, ) diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index 5e48e972b8d..2656cf32c03 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -246,3 +246,63 @@ async def test_typing_carries_live_status_phrase(): assert "content" not in typing[-1], ( "cleared phrase must omit content (empty string means CLEAR on Slack)" ) + + +# --------------------------------------------------------------------------- +# QA-1 status thread anchor: typing frames synthesize the per-message thread +# root in thread-per-message mode (the status line is thread-only on Slack). +# --------------------------------------------------------------------------- +def _wire_with_ts(chat_id, chat_type, message_id, **kw): + adapter, stub = _wire(chat_id, chat_type, **kw) + src = SessionSource( + platform=Platform.SLACK, chat_id=chat_id, chat_type=chat_type, + user_id="U1", scope_id=kw.get("scope_id"), + ) + ev = MessageEvent( + text="hi", source=src, message_type=MessageType.TEXT, message_id=message_id + ) + adapter._capture_scope(ev) + return adapter, stub + + +@pytest.mark.asyncio +async def test_typing_synthesizes_thread_anchor_in_thread_mode(): + """Top-level DM turn, thread-per-message mode: the typing frame gains the + triggering ts as thread_id so the connector's setStatus targets the + per-message thread instead of no-oping threadless.""" + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + await adapter.send_typing("D1", metadata=None) + typing = [f for f in stub.sent if f["op"] == "typing"] + assert typing and typing[-1]["metadata"].get("thread_id") == "1700.0042" + + +@pytest.mark.asyncio +async def test_typing_keeps_no_anchor_in_flat_mode(): + """Flat mode: no synthetic thread for the status either (#18859).""" + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + adapter.config.extra = {"reply_in_thread": False} + await adapter.send_typing("D1", metadata=None) + typing = [f for f in stub.sent if f["op"] == "typing"] + assert typing and "thread_id" not in typing[-1]["metadata"] + + +@pytest.mark.asyncio +async def test_typing_honours_real_thread_anchor(): + """Metadata that already names a thread wins over the synthetic cache.""" + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + await adapter.send_typing("D1", metadata={"thread_id": "1699.9000"}) + typing = [f for f in stub.sent if f["op"] == "typing"] + assert typing[-1]["metadata"]["thread_id"] == "1699.9000" + + +@pytest.mark.asyncio +async def test_stop_typing_clear_targets_same_synthesized_thread(): + """The clear frame targets the same synthesized thread as the heartbeat + (else the status line sticks).""" + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + await adapter.send_typing("D1", metadata=None) + await adapter.stop_typing("D1", metadata=None) + clears = [ + f for f in stub.sent if f["op"] == "typing" and f.get("content") == "" + ] + assert clears and clears[-1]["metadata"].get("thread_id") == "1700.0042" From a51a17ebe30960f79c7c1cddd2225e5215e27681 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 15:56:10 +0000 Subject: [PATCH 09/20] fix(relay): promote the surviving reply_to anchor into metadata.thread_id on Slack sends (QA-7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The connector's Slack sender threads on metadata ONLY: threadTs() reads metadata.thread_id/thread_ts and never the frame's reply_to. base.py's final-reply lane (and its stream-fallback 'first response' resend) builds metadata from source.thread_id — None for a top-level DM — so its sends carried reply_to as the sole threading signal and posted to the home channel (2026-07-27 post-approval report; the 15:17:03 frame showed meta_keys=['notify','user_id']). After the QA-6 mode gate keeps the anchor, copy it into metadata.thread_id so the wire carries the signal where the connector reads it. Flat mode unaffected (anchor already nulled); explicit thread metadata wins; non-Slack untouched. --- gateway/relay/adapter.py | 18 ++++++++++++++++++ .../relay/test_relay_slack_dm_streaming.py | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 8b125a22adc..4fcdb1a2ac5 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -784,6 +784,24 @@ class RelayAdapter(BasePlatformAdapter): ) if effective_reply_to is None and reply_to is not None: send_metadata.pop("reply_to_message_id", None) + # QA-7: the connector's Slack sender THREADS ON METADATA ONLY — + # threadTs() reads metadata.thread_id/thread_ts and never looks at + # the frame's reply_to. A send whose only threading signal is + # reply_to (base.py's final-reply and fallback lanes build metadata + # from source.thread_id = None for a top-level DM) would post to the + # home channel even though _resolve_reply_to_for_send kept the + # anchor. Promote the surviving anchor into metadata.thread_id so + # the wire carries it where the connector actually reads it. Only + # when the mode gate kept the anchor (thread-per-message / real + # thread) — flat mode already nulled effective_reply_to above. + if ( + effective_reply_to is not None + and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value + and not ( + send_metadata.get("thread_id") or send_metadata.get("thread_ts") + ) + ): + send_metadata["thread_id"] = str(effective_reply_to) result = await self._transport.send_outbound( { "op": "send", diff --git a/tests/gateway/relay/test_relay_slack_dm_streaming.py b/tests/gateway/relay/test_relay_slack_dm_streaming.py index 35b3d5f989c..2438609d89c 100644 --- a/tests/gateway/relay/test_relay_slack_dm_streaming.py +++ b/tests/gateway/relay/test_relay_slack_dm_streaming.py @@ -87,6 +87,10 @@ async def test_slack_dm_reply_keeps_anchor_in_thread_per_message_mode(): assert frame["reply_to"] == "1700.0001", ( "thread-per-message: the triggering ts anchors the final reply" ) + # QA-7: the connector's Slack sender threads on metadata.thread_id ONLY + # (threadTs() never reads the frame's reply_to), so the surviving anchor + # must be promoted into metadata for the send to actually thread. + assert (frame["metadata"] or {}).get("thread_id") == "1700.0001" @pytest.mark.asyncio From 71d5c47e215beda042d347d765451a6134e4e2db Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 17:35:37 +0000 Subject: [PATCH 10/20] =?UTF-8?q?fix(relay):=20per-message=20sessions=20fo?= =?UTF-8?q?r=20fronted=20Slack=20DMs=20=E2=80=94=20stamp=20the=20inbound?= =?UTF-8?q?=20ts=20as=20session=20thread=20(QA-3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 2nd top-level DM while a turn was in flight resolved to the SAME session key and steered the running turn ('Redirected current run') instead of starting its own. Native SlackAdapter stamps thread_ts = event.thread_ts or ts on EVERY inbound, so build_session_key isolates each top-level message; the connector normalizes top-level messages with thread_id=null and the relay lane never reproduced the stamp. _stamp_slack_session_thread applies native parity on the inbound bridge: top-level Slack message + thread-per-message mode => source.thread_id = its own ts (fresh session, parallel turns). Real thread replies and flat mode untouched (flat keeps the shared rolling DM session on purpose). Also introduces the enterprise config shape for relay-fronted Slack: platforms.relay.extra.slack. (nested object wins; legacy flat extra.reply_in_thread still honoured). All reply_in_thread reads (send/typing/stop_typing/run.py progress) now route through one resolver. --- gateway/relay/adapter.py | 87 +++++++++++++++---- gateway/run.py | 19 +++- .../relay/test_relay_slack_prompt_dm_root.py | 69 +++++++++++++++ 3 files changed, 153 insertions(+), 22 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 4fcdb1a2ac5..c7eaa1a981c 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -274,6 +274,7 @@ class RelayAdapter(BasePlatformAdapter): async def _on_inbound(self, event) -> None: """Bridge a connector-delivered MessageEvent into the normal adapter path.""" self._capture_scope(event) + self._stamp_slack_session_thread(event) # Phase 3: a structured prompt answer resolves its waiting primitive # (approval/confirm/clarify) and is CONSUMED — it must not also # dispatch as a chat message. Unknown/expired prompt ids fall through @@ -283,6 +284,71 @@ class RelayAdapter(BasePlatformAdapter): await self._localize_inbound_media(event) await self.handle_message(event) + def _relay_slack_extra(self) -> Dict[str, Any]: + """The Slack-behavior subset of the RELAY platform config. + + Enterprise knob shape (Hermes-config directed, relay-namespaced): + + platforms: + relay: + extra: + slack: # supported subset of native Slack fields + reply_in_thread: true + + The native ``platforms.slack`` block keeps meaning "native adapter + settings"; relay-fronted Slack reads its subset here. Legacy fallback: + a flat key on the relay extra (``extra.reply_in_thread``) still wins + when no ``slack`` object exists, preserving current staging configs. + """ + extra = getattr(self.config, "extra", None) or {} + sub = extra.get("slack") + return sub if isinstance(sub, dict) else extra + + def _effective_reply_in_thread(self) -> bool: + """Resolve the thread-per-message vs flat-DM mode for fronted Slack.""" + try: + return bool(self._relay_slack_extra().get("reply_in_thread", True)) + except Exception: # noqa: BLE001 - config shape is operator-owned + return True + + def _stamp_slack_session_thread(self, event) -> None: + """Native session-keying parity for fronted Slack (QA-3). + + Native SlackAdapter's inbound handler stamps ``thread_ts = + event.thread_ts or ts`` — every TOP-LEVEL message carries its own ts + as ``source.thread_id``, so build_session_key appends it and each + top-level message gets a FRESH session (per-message threads ⇒ + per-message sessions; a 2nd message runs parallel instead of steering + the in-flight turn). The connector normalizes a top-level message + with thread_id=null, so without this stamp every top-level DM + collapses into ONE session key and message 2 pre-empts message 1 + ("Redirected current run", 2026-07-27 report). + + Only in thread-per-message mode: flat mode keeps the shared rolling + DM session on purpose (steer/queue there is the intended UX). Never + overwrites a real thread_id (an in-thread reply must keep resolving + to its thread's session). + """ + try: + src = getattr(event, "source", None) + if not src: + return + platform = getattr(src, "platform", None) + if getattr(platform, "value", platform) != Platform.SLACK.value: + return + if getattr(src, "thread_id", None): + return # real thread — its session key is already correct + message_id = getattr(event, "message_id", None) or getattr( + src, "message_id", None + ) + if not message_id: + return + if not self._effective_reply_in_thread(): + return + src.thread_id = str(message_id) + except Exception: # noqa: BLE001 - session stamping must never break inbound + logger.debug("slack session-thread stamp failed", exc_info=True) + async def _localize_inbound_media(self, event) -> None: """Download connector re-hosted attachments to local temp paths. @@ -875,12 +941,7 @@ class RelayAdapter(BasePlatformAdapter): # message to the DM root while progress stayed threaded (2026-07-27 # report, sibling of the QA-5 prompt bug). Native SlackAdapter only # suppresses the anchor when reply_in_thread=false; mirror that. - try: - reply_in_thread = bool( - (self.config.extra or {}).get("reply_in_thread", True) - ) - except Exception: # noqa: BLE001 - config shape is adapter-owned - reply_in_thread = True + reply_in_thread = self._effective_reply_in_thread() if reply_in_thread: # Thread-per-message: the triggering ts is the thread anchor. return reply_to @@ -962,12 +1023,7 @@ class RelayAdapter(BasePlatformAdapter): and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): - try: - reply_in_thread = bool( - (self.config.extra or {}).get("reply_in_thread", True) - ) - except Exception: # noqa: BLE001 - config shape is adapter-owned - reply_in_thread = True + reply_in_thread = self._effective_reply_in_thread() anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) if reply_in_thread and anchor: md["thread_id"] = anchor @@ -1028,12 +1084,7 @@ class RelayAdapter(BasePlatformAdapter): not (md.get("thread_id") or md.get("thread_ts")) and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): - try: - reply_in_thread = bool( - (self.config.extra or {}).get("reply_in_thread", True) - ) - except Exception: # noqa: BLE001 - config shape is adapter-owned - reply_in_thread = True + reply_in_thread = self._effective_reply_in_thread() anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) if reply_in_thread and anchor: md["thread_id"] = anchor diff --git a/gateway/run.py b/gateway/run.py index aac6a192555..5e7f159011e 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -20688,11 +20688,22 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew _slack_adapter_for_progress = self._adapter_for_source(source) if _slack_adapter_for_progress is not None: try: - _progress_reply_in_thread = bool( - _slack_adapter_for_progress.config.extra.get( - "reply_in_thread", True - ) + # Relay lane: the adapter owns mode resolution (nested + # platforms.relay.extra.slack subset with flat-key + # fallback). Native lane: read the flat extra as before. + _mode_fn = getattr( + _slack_adapter_for_progress, + "_effective_reply_in_thread", + None, ) + if callable(_mode_fn): + _progress_reply_in_thread = bool(_mode_fn()) + else: + _progress_reply_in_thread = bool( + _slack_adapter_for_progress.config.extra.get( + "reply_in_thread", True + ) + ) except Exception: _progress_reply_in_thread = True _progress_thread_id = _resolve_progress_thread_id( diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index 2656cf32c03..098b1fcadde 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -306,3 +306,72 @@ async def test_stop_typing_clear_targets_same_synthesized_thread(): f for f in stub.sent if f["op"] == "typing" and f.get("content") == "" ] assert clears and clears[-1]["metadata"].get("thread_id") == "1700.0042" + + +# --------------------------------------------------------------------------- +# QA-3 session keying: a top-level Slack DM message gets its own ts stamped as +# source.thread_id (native inbound parity) so each message keys a FRESH +# session in thread-per-message mode; flat mode and real threads untouched. +# --------------------------------------------------------------------------- +def _inbound_event(chat_id="D1", message_id="1700.0100", thread_id=None): + src = SessionSource( + platform=Platform.SLACK, chat_id=chat_id, chat_type="dm", + user_id="U1", scope_id="T1", thread_id=thread_id, + ) + return MessageEvent( + text="hi", source=src, message_type=MessageType.TEXT, + message_id=message_id, + ) + + +def test_top_level_dm_gets_session_thread_stamp(): + adapter, _ = _wire("D1", "dm") + ev = _inbound_event(message_id="1700.0100") + adapter._stamp_slack_session_thread(ev) + assert ev.source.thread_id == "1700.0100" + + +def test_two_top_level_messages_key_distinct_sessions(): + from gateway.session import build_session_key + adapter, _ = _wire("D1", "dm") + e1 = _inbound_event(message_id="1700.0100") + e2 = _inbound_event(message_id="1700.0200") + adapter._stamp_slack_session_thread(e1) + adapter._stamp_slack_session_thread(e2) + k1 = build_session_key(e1.source) + k2 = build_session_key(e2.source) + assert k1 != k2, "each top-level message must be its own session (QA-3)" + + +def test_real_thread_reply_keeps_its_thread_session(): + adapter, _ = _wire("D1", "dm") + ev = _inbound_event(message_id="1700.0300", thread_id="1700.0100") + adapter._stamp_slack_session_thread(ev) + assert ev.source.thread_id == "1700.0100", ( + "an in-thread reply must keep resolving to its thread's session" + ) + + +def test_flat_mode_keeps_shared_dm_session(): + adapter, _ = _wire("D1", "dm") + adapter.config.extra = {"reply_in_thread": False} + ev = _inbound_event(message_id="1700.0400") + adapter._stamp_slack_session_thread(ev) + assert ev.source.thread_id is None, ( + "flat mode: shared rolling DM session (steer/queue) is intended UX" + ) + + +def test_nested_relay_slack_config_subset_wins(): + """Enterprise knob shape: platforms.relay.extra.slack.reply_in_thread.""" + adapter, _ = _wire("D1", "dm") + adapter.config.extra = {"slack": {"reply_in_thread": False}} + assert adapter._effective_reply_in_thread() is False + adapter.config.extra = {"slack": {"reply_in_thread": True}} + assert adapter._effective_reply_in_thread() is True + # Legacy flat key still honoured when no nested object exists. + adapter.config.extra = {"reply_in_thread": False} + assert adapter._effective_reply_in_thread() is False + # Default: thread-per-message. + adapter.config.extra = {} + assert adapter._effective_reply_in_thread() is True From 9864e00fb42a4c7f49b2767dd58a8a0a1f832865 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 27 Jul 2026 22:59:02 +0000 Subject: [PATCH 11/20] =?UTF-8?q?feat(relay):=20flat-DM=20liveliness=20?= =?UTF-8?q?=E2=80=94=20status=20anchors=20to=20the=20triggering=20ts,=20re?= =?UTF-8?q?plies=20stay=20flat=20(QA-8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Victor's correction: flat DMs CAN have a live thinking status. setStatus on the triggering message's ts renders '… thinking'/per-tool phrases in that message's thread-footer space and clears without leaving a message artifact. Native suppresses this because ITS reply routing could inherit the activated thread; the relay lane's flat-mode sends strip their anchors explicitly (QA-6/7), so the status anchor cannot leak into reply placement — proven by the new leak-guard test. send_typing/stop_typing now anchor the status in flat mode too, gated by platforms.relay.extra.slack.flat_dm_status (default ON; false restores the fully anchorless posture). Thread mode unchanged. --- gateway/relay/adapter.py | 36 ++++++++++++++++--- .../relay/test_relay_slack_prompt_dm_root.py | 31 ++++++++++++++-- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index c7eaa1a981c..4651ee1c34e 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -311,6 +311,24 @@ class RelayAdapter(BasePlatformAdapter): except Exception: # noqa: BLE001 - config shape is operator-owned return True + def _flat_dm_status_enabled(self) -> bool: + """Liveliness in flat-DM mode: anchor the STATUS (not the reply) to the + triggering message's ts. + + ``assistant.threads.setStatus`` on a message ts renders "… thinking" in + that message's thread-footer space and vanishes on clear — no message + artifact. Native suppresses this in flat mode because ITS response + routing could inherit the activated thread; the relay lane's sends are + explicitly flat in flat mode (QA-6/7 anchor strip), so the status + anchor cannot leak into reply placement here. Default ON — flat DMs + get a live status billboard while replies still post at the DM root. + Opt out: platforms.relay.extra.slack.flat_dm_status: false. + """ + try: + return bool(self._relay_slack_extra().get("flat_dm_status", True)) + except Exception: # noqa: BLE001 - config shape is operator-owned + return True + def _stamp_slack_session_thread(self, event) -> None: """Native session-keying parity for fronted Slack (QA-3). @@ -1023,9 +1041,17 @@ class RelayAdapter(BasePlatformAdapter): and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): - reply_in_thread = self._effective_reply_in_thread() anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) - if reply_in_thread and anchor: + # Thread mode: status targets the per-message thread (QA-1). + # Flat mode: the status can STILL anchor to the triggering ts — + # setStatus renders in the footer space and clears without a + # message artifact, and flat sends strip their anchors (QA-6/7) + # so reply placement cannot inherit it. Gated separately + # (flat_dm_status, default on) for a clean opt-out. + if anchor and ( + self._effective_reply_in_thread() + or self._flat_dm_status_enabled() + ): md["thread_id"] = anchor # Rich status parity (QA-1): run.py's live-status lane stashes the # current per-tool phrase via set_status_text() (base class store). @@ -1084,9 +1110,11 @@ class RelayAdapter(BasePlatformAdapter): not (md.get("thread_id") or md.get("thread_ts")) and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): - reply_in_thread = self._effective_reply_in_thread() anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) - if reply_in_thread and anchor: + if anchor and ( + self._effective_reply_in_thread() + or self._flat_dm_status_enabled() + ): md["thread_id"] = anchor try: await self._transport.send_outbound( diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index 098b1fcadde..d1c062075b3 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -277,15 +277,42 @@ async def test_typing_synthesizes_thread_anchor_in_thread_mode(): @pytest.mark.asyncio -async def test_typing_keeps_no_anchor_in_flat_mode(): - """Flat mode: no synthetic thread for the status either (#18859).""" +async def test_typing_flat_mode_status_anchors_to_trigger_ts_by_default(): + """Flat-DM liveliness: the STATUS still anchors to the triggering ts + (renders in the footer space, no message artifact) while replies stay + flat — QA-6/7 strip send anchors, so placement cannot inherit this.""" adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") adapter.config.extra = {"reply_in_thread": False} await adapter.send_typing("D1", metadata=None) typing = [f for f in stub.sent if f["op"] == "typing"] + assert typing and typing[-1]["metadata"].get("thread_id") == "1700.0042" + + +@pytest.mark.asyncio +async def test_typing_flat_mode_opt_out_drops_anchor(): + """flat_dm_status: false restores the fully-anchorless flat posture.""" + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + adapter.config.extra = { + "slack": {"reply_in_thread": False, "flat_dm_status": False} + } + await adapter.send_typing("D1", metadata=None) + typing = [f for f in stub.sent if f["op"] == "typing"] assert typing and "thread_id" not in typing[-1]["metadata"] +@pytest.mark.asyncio +async def test_flat_mode_sends_stay_flat_with_status_anchor_active(): + """The liveliness anchor must NOT leak into reply placement: sends in + flat mode still strip the synthetic anchor (QA-6/7 contract).""" + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + adapter.config.extra = {"reply_in_thread": False} + await adapter.send_typing("D1", metadata=None) + await adapter.send("D1", "the answer", reply_to="1700.0042") + frame = [f for f in stub.sent if f["op"] == "send"][-1] + assert frame["reply_to"] is None + assert "thread_id" not in (frame["metadata"] or {}) + + @pytest.mark.asyncio async def test_typing_honours_real_thread_anchor(): """Metadata that already names a thread wins over the synthetic cache.""" From 85a75f3155c19de8ddeca9804567b2e128f5e1a3 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Tue, 28 Jul 2026 00:05:41 +0000 Subject: [PATCH 12/20] =?UTF-8?q?refactor(relay):=20drop=20the=20flat=5Fdm?= =?UTF-8?q?=5Fstatus=20knob=20=E2=80=94=20liveliness=20is=20unconditional;?= =?UTF-8?q?=20add=20relay=20docs=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flat_dm_status was speculative config (rubric violation): no user wants 'make my agent look dead', and the only real consumer of status suppression was native's placement-contamination guard — which the relay lane handles structurally (QA-6/7 send-side anchor strip, leak-guard test), not via preference. Status now anchors whenever an inbound ts exists, in both modes. Docs: new website/docs/user-guide/messaging/relay.md — enterprise-only relay lane page documenting the platforms.relay.extra. subset shape (nested wins, flat fallback), the Slack reply_in_thread control, and always-on liveliness. Kept out of the native slack.md on purpose: relay controls are not Slack config. --- gateway/relay/adapter.py | 37 +++---------- .../relay/test_relay_slack_prompt_dm_root.py | 19 ++++--- website/docs/user-guide/messaging/relay.md | 55 +++++++++++++++++++ 3 files changed, 72 insertions(+), 39 deletions(-) create mode 100644 website/docs/user-guide/messaging/relay.md diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 4651ee1c34e..1654e988071 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -311,24 +311,6 @@ class RelayAdapter(BasePlatformAdapter): except Exception: # noqa: BLE001 - config shape is operator-owned return True - def _flat_dm_status_enabled(self) -> bool: - """Liveliness in flat-DM mode: anchor the STATUS (not the reply) to the - triggering message's ts. - - ``assistant.threads.setStatus`` on a message ts renders "… thinking" in - that message's thread-footer space and vanishes on clear — no message - artifact. Native suppresses this in flat mode because ITS response - routing could inherit the activated thread; the relay lane's sends are - explicitly flat in flat mode (QA-6/7 anchor strip), so the status - anchor cannot leak into reply placement here. Default ON — flat DMs - get a live status billboard while replies still post at the DM root. - Opt out: platforms.relay.extra.slack.flat_dm_status: false. - """ - try: - return bool(self._relay_slack_extra().get("flat_dm_status", True)) - except Exception: # noqa: BLE001 - config shape is operator-owned - return True - def _stamp_slack_session_thread(self, event) -> None: """Native session-keying parity for fronted Slack (QA-3). @@ -1041,17 +1023,15 @@ class RelayAdapter(BasePlatformAdapter): and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): - anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) # Thread mode: status targets the per-message thread (QA-1). - # Flat mode: the status can STILL anchor to the triggering ts — + # Flat mode: the status STILL anchors to the triggering ts — # setStatus renders in the footer space and clears without a # message artifact, and flat sends strip their anchors (QA-6/7) - # so reply placement cannot inherit it. Gated separately - # (flat_dm_status, default on) for a clean opt-out. - if anchor and ( - self._effective_reply_in_thread() - or self._flat_dm_status_enabled() - ): + # so reply placement cannot inherit it. Unconditional: liveliness + # is not a preference, it ships in whatever form the mode + # supports (no speculative opt-out knob). + anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) + if anchor: md["thread_id"] = anchor # Rich status parity (QA-1): run.py's live-status lane stashes the # current per-tool phrase via set_status_text() (base class store). @@ -1111,10 +1091,7 @@ class RelayAdapter(BasePlatformAdapter): and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) - if anchor and ( - self._effective_reply_in_thread() - or self._flat_dm_status_enabled() - ): + if anchor: md["thread_id"] = anchor try: await self._transport.send_outbound( diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index d1c062075b3..7d2c630c07d 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -289,15 +289,16 @@ async def test_typing_flat_mode_status_anchors_to_trigger_ts_by_default(): @pytest.mark.asyncio -async def test_typing_flat_mode_opt_out_drops_anchor(): - """flat_dm_status: false restores the fully-anchorless flat posture.""" - adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") - adapter.config.extra = { - "slack": {"reply_in_thread": False, "flat_dm_status": False} - } - await adapter.send_typing("D1", metadata=None) - typing = [f for f in stub.sent if f["op"] == "typing"] - assert typing and "thread_id" not in typing[-1]["metadata"] +async def test_typing_anchors_unconditionally_in_both_modes(): + """Liveliness is not a preference: the status anchors whenever an inbound + ts exists, regardless of reply_in_thread. Placement safety comes from the + QA-6/7 send-side anchor strip, not from suppressing the status.""" + for extra in ({}, {"slack": {"reply_in_thread": False}}): + adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") + adapter.config.extra = extra + await adapter.send_typing("D1", metadata=None) + typing = [f for f in stub.sent if f["op"] == "typing"] + assert typing and typing[-1]["metadata"].get("thread_id") == "1700.0042" @pytest.mark.asyncio diff --git a/website/docs/user-guide/messaging/relay.md b/website/docs/user-guide/messaging/relay.md new file mode 100644 index 00000000000..22a98c1a149 --- /dev/null +++ b/website/docs/user-guide/messaging/relay.md @@ -0,0 +1,55 @@ +# Relay (Team Gateway) — Enterprise + +> **Enterprise-only.** The relay lane applies when your Hermes gateway is +> fronted by a [Team Gateway connector](https://github.com/NousResearch/gateway-gateway) +> — the enterprise deployment model where the connector owns the platform +> credentials (e.g. one org Slack app) and Hermes speaks a relay protocol to +> it instead of connecting to the platform natively. Standalone/native +> installs can ignore this page; your platform's own page (e.g. +> [Slack](slack.md)) applies instead. + +## How configuration works on the relay lane + +The relay adapter is platform-neutral: the connector tells Hermes which +platform it fronts, and Hermes expresses per-turn decisions (threading, +status, placement) as frame metadata the connector executes mechanically. + +A small set of **platform behavior controls** exist for the fronted platform. +They live under `platforms.relay.extra.` — a supported *subset* of +that platform's native options — NOT under the native platform block: + +```yaml +platforms: + slack: # native adapter settings — ignored on the relay lane + ... + relay: + extra: + slack: # relay-lane subset for fronted Slack + reply_in_thread: true +``` + +Resolution order: the nested `extra.` object wins → a legacy flat +key on `extra` is honored as a fallback → the option's default. + +## Supported controls — Slack + +| Key | Default | Effect | +|---|---|---| +| `reply_in_thread` | `true` | `true`: thread-per-message — each top-level DM message opens its own thread carrying the entire turn (status, tool progress, approval cards, final reply), and each message runs as its own session so concurrent messages execute in parallel. `false`: flat rolling DM — everything posts at the DM root, one shared session, a second message steers the in-flight turn. | + +Semantics match the native Slack adapter's `reply_in_thread` +([Slack docs](slack.md)); the relay subset exists so relay-fronted behavior +is configured explicitly rather than inherited from a native block that the +relay lane does not read. + +In-progress "thinking…" statuses (with live per-tool phrases) are always on, +in whatever form the mode supports: the thread's replies footer in +thread-per-message mode, or anchored to the triggering message in flat mode. +They require only the `chat:write` bot scope — no Slack assistant surface. + +Changing a control takes effect on gateway restart — `hermes config set +platforms.relay.extra.slack.reply_in_thread false` and restart; no connector +deploy is involved. + +Other fronted platforms currently have no relay-lane controls; the set grows +as enterprise deployments need them (each addition is documented here). From daefa8c34ed9df8f94e1ccc7ddf1878af107a54f Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Tue, 28 Jul 2026 00:12:00 +0000 Subject: [PATCH 13/20] docs(relay): move behavior-controls docs into the relay-connector contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relocate the platforms.relay.extra. documentation from a new user-guide page into docs/relay-connector-contract.md (the existing canonical relay doc, already linked from gateway-internals) as §8. The relay lane is an enterprise-only component: it gets minor coverage in the developer-facing contract doc, not a prominent user-guide page, and no links to private components. --- docs/relay-connector-contract.md | 39 ++++++++++++++- website/docs/user-guide/messaging/relay.md | 55 ---------------------- 2 files changed, 38 insertions(+), 56 deletions(-) delete mode 100644 website/docs/user-guide/messaging/relay.md diff --git a/docs/relay-connector-contract.md b/docs/relay-connector-contract.md index 3698b55432a..e13d51459dd 100644 --- a/docs/relay-connector-contract.md +++ b/docs/relay-connector-contract.md @@ -711,7 +711,44 @@ per-gateway secret and the same host as `/relay/provision`. --- -## 8. Versioning policy +## 8. Gateway-side platform behavior controls (enterprise) + +Enterprise deployments configure fronted-platform behavior on the GATEWAY +side, under `platforms.relay.extra.` — a supported subset of that +platform's native options. The native platform block (e.g. `platforms.slack`) +is not read on the relay lane; the connector receives the *outcome* of these +controls as frame metadata (§4) and executes mechanically — it holds no +platform behavior policy of its own. + +```yaml +platforms: + relay: + extra: + slack: + reply_in_thread: true # default +``` + +Resolution: nested `extra.` object wins → legacy flat key on +`extra` honored as fallback → default. Source of truth: +`RelayAdapter._effective_reply_in_thread` (`gateway/relay/adapter.py`). + +Current controls (Slack): + +| Key | Default | Effect | +| --- | --- | --- | +| `reply_in_thread` | `true` | `true`: thread-per-message — each top-level DM message anchors its own thread (status, progress, prompts, final reply all carry that `metadata.thread_id`) and keys its own session, so concurrent messages run in parallel. `false`: flat rolling DM — send-lane frames carry NO thread anchor (stripped, not omitted), one shared session per DM. | + +Typing/status frames always carry the triggering-ts anchor when one is known +(liveliness is unconditional, both modes): Slack's status line is +thread-scoped, and in flat mode the send-side anchor strip guarantees the +status anchor can never leak into reply placement. Semantics of the native +key: see `website/docs/user-guide/messaging/slack.md`. + +Changes take effect on gateway restart; no connector involvement. + +--- + +## 9. Versioning policy - `contract_version` is an int; bump **only** for additive changes during the experimental phase (new optional fields, new `op`s). diff --git a/website/docs/user-guide/messaging/relay.md b/website/docs/user-guide/messaging/relay.md deleted file mode 100644 index 22a98c1a149..00000000000 --- a/website/docs/user-guide/messaging/relay.md +++ /dev/null @@ -1,55 +0,0 @@ -# Relay (Team Gateway) — Enterprise - -> **Enterprise-only.** The relay lane applies when your Hermes gateway is -> fronted by a [Team Gateway connector](https://github.com/NousResearch/gateway-gateway) -> — the enterprise deployment model where the connector owns the platform -> credentials (e.g. one org Slack app) and Hermes speaks a relay protocol to -> it instead of connecting to the platform natively. Standalone/native -> installs can ignore this page; your platform's own page (e.g. -> [Slack](slack.md)) applies instead. - -## How configuration works on the relay lane - -The relay adapter is platform-neutral: the connector tells Hermes which -platform it fronts, and Hermes expresses per-turn decisions (threading, -status, placement) as frame metadata the connector executes mechanically. - -A small set of **platform behavior controls** exist for the fronted platform. -They live under `platforms.relay.extra.` — a supported *subset* of -that platform's native options — NOT under the native platform block: - -```yaml -platforms: - slack: # native adapter settings — ignored on the relay lane - ... - relay: - extra: - slack: # relay-lane subset for fronted Slack - reply_in_thread: true -``` - -Resolution order: the nested `extra.` object wins → a legacy flat -key on `extra` is honored as a fallback → the option's default. - -## Supported controls — Slack - -| Key | Default | Effect | -|---|---|---| -| `reply_in_thread` | `true` | `true`: thread-per-message — each top-level DM message opens its own thread carrying the entire turn (status, tool progress, approval cards, final reply), and each message runs as its own session so concurrent messages execute in parallel. `false`: flat rolling DM — everything posts at the DM root, one shared session, a second message steers the in-flight turn. | - -Semantics match the native Slack adapter's `reply_in_thread` -([Slack docs](slack.md)); the relay subset exists so relay-fronted behavior -is configured explicitly rather than inherited from a native block that the -relay lane does not read. - -In-progress "thinking…" statuses (with live per-tool phrases) are always on, -in whatever form the mode supports: the thread's replies footer in -thread-per-message mode, or anchored to the triggering message in flat mode. -They require only the `chat:write` bot scope — no Slack assistant surface. - -Changing a control takes effect on gateway restart — `hermes config set -platforms.relay.extra.slack.reply_in_thread false` and restart; no connector -deploy is involved. - -Other fronted platforms currently have no relay-lane controls; the set grows -as enterprise deployments need them (each addition is documented here). From a09015d31e29cac5389fac8eb662b49c0645a8d1 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Tue, 28 Jul 2026 11:42:40 +0000 Subject: [PATCH 14/20] Revert "fix(scripts): encode tool_search_livetest2 output as utf-8 (Windows footgun)" This reverts commit e286658377ed63f17582be59bdc081811ed63b3d. --- scripts/tool_search_livetest2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tool_search_livetest2.py b/scripts/tool_search_livetest2.py index 522b141685c..b81f91f91af 100644 --- a/scripts/tool_search_livetest2.py +++ b/scripts/tool_search_livetest2.py @@ -187,7 +187,7 @@ def run_one(scenario: Dict[str, Any], mode: str, rep: int, out_dir: Path) -> Dic "final_response": base._redact_secrets(final_response)[:500], } out_path = out_dir / f"{scenario['id']}__{'enabled' if enabled else 'disabled'}__rep{rep}.json" - out_path.write_text(json.dumps(rec, indent=1), encoding="utf-8") + out_path.write_text(json.dumps(rec, indent=1)) shutil.rmtree(Path(os.environ["HERMES_HOME"]).parent, ignore_errors=True) return rec From 09c4a1d34917d319d2f3f55a6ff3dd023a29a39f Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Tue, 28 Jul 2026 11:33:01 +0000 Subject: [PATCH 15/20] refactor(relay): remove dead _strip_synthetic_dm_thread; pin the run.py anchor-suppression boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (2026-07-28): every path through _strip_synthetic_dm_thread returned metadata unmodified — the actual strip was removed when prompts switched to trusting the run.py thread stamp, leaving a 50-line no-op and four tests that passed against it (verified by reviewer's negative control). - delete the function + its _send_prompt call site (verbatim pass-through with a pointer comment to the single mode authority) - rewrite the three pass-through tests as end-to-end placement contracts (forward run.py's stamp untouched) - NEW boundary tests pinning run.py._resolve_progress_thread_id itself: flat mode suppresses the synthetic self-anchor / preserves real threads; thread mode keeps the first-turn self-anchor. This is the cross-module coupling the review flagged as unpinned — if the upstream suppression regresses, these fail instead of prompts silently threading. --- gateway/relay/adapter.py | 61 ++--------------- .../relay/test_relay_slack_prompt_dm_root.py | 65 ++++++++++++++++--- 2 files changed, 61 insertions(+), 65 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 1654e988071..646f720a9bd 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -1448,61 +1448,6 @@ class RelayAdapter(BasePlatformAdapter): return None return state - def _strip_synthetic_dm_thread( - self, chat_id: str, metadata: Optional[Dict[str, Any]] - ) -> Optional[Dict[str, Any]]: - """Drop the synthetic DM thread anchor from an interactive prompt's metadata. - - A clarify/approval/confirm prompt is emitted mid-turn in reply to the - triggering inbound event, so ``metadata`` carries that event's thread - context — run.py's ``_thread_metadata_for_source`` stamps - ``metadata["thread_id"]`` (and, for Slack, ``metadata["message_id"]`` = - the triggering message ts). For a Slack DM with no REAL thread, that - ``thread_id`` is the message's own synthetic self-anchor (a session-keying - fallback), and forwarding it makes the connector's slackRestSender thread - the prompt card UNDER the user's message instead of posting it flat at the - DM root — the reported bug ("approval block was put in a thread"). - - Native Slack Hermes already suppresses this synthetic DM thread anchor - (``SlackAdapter._resolve_thread_ts`` returns ``None`` for a top-level / DM - message). We reproduce it here with the same discipline used on the - streaming path (``_resolve_reply_to_for_send``): - - Slack DM + thread_id is the synthetic self-anchor ⇒ strip thread_id. - - A REAL thread (``thread_id`` distinct from the triggering message ts) is - left untouched so a prompt raised inside a thread stays in that thread; - non-DM / non-Slack chats are never matched. Only the threading keys are - removed — tenant scope (``scope_id`` / ``slack_team_id``) and everything - else survive so egress routing is unaffected. - """ - if not metadata: - return metadata - if self._platform_by_chat.get(str(chat_id)) != Platform.SLACK.value: - return metadata - if self._chat_type_by_chat.get(str(chat_id)) != "dm": - return metadata - thread_id = metadata.get("thread_id") - if not thread_id: - return metadata - # Trust the run.py stamp (QA-5). The threading MODE is decided in ONE - # place — run.py's _resolve_progress_thread_id, which reads - # platforms.slack.extra.reply_in_thread: - # * flat mode (reply_in_thread=false): the synthetic self-anchor is - # suppressed THERE, so prompt metadata arrives with NO thread_id and - # this helper is a no-op — the card posts flat at the DM root; - # * thread-per-message mode (default): metadata.thread_id is stamped - # for the whole turn, and on the FIRST turn it legitimately equals - # the triggering message's ts (the synthetic root IS the thread). - # The previous unconditional thread_id == message_id strip re-derived - # the mode here and got it wrong for thread-per-message: the approval - # card (and its resolved-state swap) was exiled to the DM root while - # progress bubbles honoured the thread (2026-07-27 mixed-placement - # screenshot). Mirror native SlackAdapter._resolve_thread_ts, which - # only performs the self-anchor strip when reply_in_thread=false — a - # state this lane never sees with an anchor present, per the above. - return metadata - async def _send_prompt( self, chat_id: str, @@ -1533,7 +1478,11 @@ class RelayAdapter(BasePlatformAdapter): # of posting it flat at the DM root (the reported bug). Native Slack # Hermes suppresses this synthetic DM thread anchor; drop it here for the # same Slack-DM-with-no-real-thread case, matching _resolve_reply_to_for_send. - prompt_metadata = self._strip_synthetic_dm_thread(chat_id, metadata) + # Prompt metadata is forwarded VERBATIM. The threading mode is decided + # in exactly one place — run.py's _resolve_progress_thread_id (flat mode + # suppresses the synthetic self-anchor there; thread mode stamps the + # turn's thread). Boundary pinned by test_run_py_suppresses_self_anchor*. + prompt_metadata = metadata action: Dict[str, Any] = { "op": "prompt", "chat_id": chat_id, diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index 7d2c630c07d..eabc3b2d412 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -110,14 +110,16 @@ async def test_exec_approval_flat_mode_posts_at_dm_root(): # --------------------------------------------------------------------------- -# Thread-per-message mode: the first-turn self-anchor (thread_id == message_id) -# IS the thread root — the prompt must stay in the thread (QA-5 regression). +# Thread-per-message mode, end-to-end placement contract: run.py stamps the +# turn's thread (first turn: the triggering message's own ts) and the adapter +# forwards prompt metadata UNTOUCHED — no re-derivation, no strip. Mixed +# placement (progress threaded, card at root) was the 2026-07-27 regression. # --------------------------------------------------------------------------- @pytest.mark.asyncio -async def test_exec_approval_first_turn_self_anchor_stays_in_thread(): - """Thread-per-message first turn: run.py stamps thread_id = the triggering - message's own ts. The approval card must post INTO that thread — stripping - it exiled the card to the home channel (2026-07-27 report).""" +async def test_exec_approval_forwards_run_py_thread_stamp_untouched(): + """The adapter must forward run.py's thread stamp verbatim: the approval + card posts INTO the stamped thread. Any adapter-side re-derivation or + strip exiled the card to the home channel (2026-07-27 report).""" adapter, stub = _wire("D1", "dm", scope_id="T1") md = { "thread_id": "1700000000.000100", @@ -137,7 +139,7 @@ async def test_exec_approval_first_turn_self_anchor_stays_in_thread(): @pytest.mark.asyncio -async def test_clarify_first_turn_self_anchor_stays_in_thread(): +async def test_clarify_forwards_run_py_thread_stamp_untouched(): adapter, stub = _wire("D1", "dm", scope_id="T1") md = { "thread_id": "1700000000.000200", @@ -155,8 +157,8 @@ async def test_clarify_first_turn_self_anchor_stays_in_thread(): @pytest.mark.asyncio -async def test_slash_confirm_first_turn_self_anchor_stays_in_thread(): - """The stamp-trusting rule covers every prompt surface (single +async def test_slash_confirm_forwards_run_py_thread_stamp_untouched(): + """The forward-untouched rule covers every prompt surface (single _send_prompt choke point).""" adapter, stub = _wire("D1", "dm") md = {"thread_id": "1700000000.000300", "message_id": "1700000000.000300"} @@ -403,3 +405,48 @@ def test_nested_relay_slack_config_subset_wins(): # Default: thread-per-message. adapter.config.extra = {} assert adapter._effective_reply_in_thread() is True + + +# --------------------------------------------------------------------------- +# Cross-module boundary pin (review 2026-07-28): the adapter deliberately has +# NO prompt-side strip — flat-mode placement depends entirely on run.py's +# _resolve_progress_thread_id suppressing the synthetic self-anchor upstream. +# If that suppression regresses, prompt cards silently thread again. These +# tests pin the boundary in BOTH modes so the coupling is load-bearing. +# --------------------------------------------------------------------------- +def test_run_py_suppresses_self_anchor_in_flat_mode(): + from gateway.run import _resolve_progress_thread_id + + # Flat mode + synthetic self-anchor (thread_id == own message id) => None: + # prompt/progress metadata arrives at the adapter with NO thread anchor. + assert ( + _resolve_progress_thread_id( + "slack", "1700.001", "1700.001", reply_in_thread=False + ) + is None + ) + # Flat mode + REAL thread (ids differ) => the real thread survives. + assert ( + _resolve_progress_thread_id( + "slack", "1699.000", "1700.001", reply_in_thread=False + ) + == "1699.000" + ) + + +def test_run_py_keeps_self_anchor_in_thread_mode(): + from gateway.run import _resolve_progress_thread_id + + # Thread-per-message mode: the first-turn self-anchor IS the thread root + # and must flow through to the adapter unchanged. + assert ( + _resolve_progress_thread_id( + "slack", "1700.001", "1700.001", reply_in_thread=True + ) + == "1700.001" + ) + # No source thread at all: Slack synthesizes the root from the message id. + assert ( + _resolve_progress_thread_id("slack", None, "1700.001", reply_in_thread=True) + == "1700.001" + ) From 3e628edeb928581d83cd6c34a3cc3b3911b046d9 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Tue, 28 Jul 2026 11:33:25 +0000 Subject: [PATCH 16/20] docs(relay): replace internal QA-N tracker markers with behavior descriptions Review finding: QA-1/3/5/6/7 are internal campaign tracker ids meaning nothing to future readers of this file. Comments now describe the behavior (status thread targeting, metadata-only threading, session-keying parity) instead of citing the tracker. Comment-only change. --- gateway/relay/adapter.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 646f720a9bd..8e127ac1f07 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -83,7 +83,7 @@ class RelayAdapter(BasePlatformAdapter): # disambiguation, and it needs the chat_type to know a chat is a DM. self._chat_type_by_chat: Dict[str, str] = {} # chat_id -> last triggering message ts (Slack). The typing/status - # lane's synthetic thread anchor in thread-per-message mode (QA-1); + # lane's synthetic thread anchor in thread-per-message mode; # see _capture_scope and send_typing. self._last_inbound_ts_by_chat: Dict[str, str] = {} # chat_id -> the UNDERLYING platform (e.g. "discord", "telegram") this @@ -141,7 +141,7 @@ class RelayAdapter(BasePlatformAdapter): def supports_status_text(self) -> bool: # type: ignore[override] """Whether the fronted platform renders a TEXT status line. - Native parity (QA-1 rich status): Slack's typing surface is the + Native parity (rich status text): Slack's typing surface is the assistant status line ("Finding answers…" next to the bot name), a text-rendering indicator. When the relay fronts Slack, advertise it so run.py's live-status lane feeds per-tool phrases via @@ -312,7 +312,7 @@ class RelayAdapter(BasePlatformAdapter): return True def _stamp_slack_session_thread(self, event) -> None: - """Native session-keying parity for fronted Slack (QA-3). + """Native session-keying parity for fronted Slack DMs. Native SlackAdapter's inbound handler stamps ``thread_ts = event.thread_ts or ts`` — every TOP-LEVEL message carries its own ts @@ -448,7 +448,7 @@ class RelayAdapter(BasePlatformAdapter): chat_type = getattr(src, "chat_type", None) if chat_type: self._chat_type_by_chat[str(chat)] = str(chat_type) - # Triggering message ts (QA-1): the typing/status lane's metadata + # Triggering message ts: the typing/status lane's metadata # (base.py _thread_metadata_for_source) carries NO thread anchor # for a top-level DM, but in thread-per-message mode the status # must target the per-message thread (its root = this ts). Cache @@ -850,7 +850,7 @@ class RelayAdapter(BasePlatformAdapter): ) if effective_reply_to is None and reply_to is not None: send_metadata.pop("reply_to_message_id", None) - # QA-7: the connector's Slack sender THREADS ON METADATA ONLY — + # The connector's Slack sender THREADS ON METADATA ONLY — # threadTs() reads metadata.thread_id/thread_ts and never looks at # the frame's reply_to. A send whose only threading signal is # reply_to (base.py's final-reply and fallback lanes build metadata @@ -939,7 +939,7 @@ class RelayAdapter(BasePlatformAdapter): # threading signal (run.py's synthetic root feeds just the # progress/status lane). Dropping it unconditionally exiled the final # message to the DM root while progress stayed threaded (2026-07-27 - # report, sibling of the QA-5 prompt bug). Native SlackAdapter only + # report, same class as the prompt-placement bug). Native SlackAdapter only # suppresses the anchor when reply_in_thread=false; mirror that. reply_in_thread = self._effective_reply_in_thread() if reply_in_thread: @@ -1005,7 +1005,7 @@ class RelayAdapter(BasePlatformAdapter): """ if self._transport is None: return - # Thread anchor for the status surface (QA-1). Slack's status line + # Thread anchor for the status surface. Slack's status line # ("is thinking…" in the thread's replies footer — works with plain # chat:write, confirmed on native no-assistant bots) is THREAD-only: # the connector's typing case no-ops without a thread_ts. But the @@ -1023,17 +1023,17 @@ class RelayAdapter(BasePlatformAdapter): and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value and self._chat_type_by_chat.get(str(chat_id)) == "dm" ): - # Thread mode: status targets the per-message thread (QA-1). + # Thread mode: status targets the per-message thread. # Flat mode: the status STILL anchors to the triggering ts — # setStatus renders in the footer space and clears without a - # message artifact, and flat sends strip their anchors (QA-6/7) + # message artifact, and flat sends strip their anchors # so reply placement cannot inherit it. Unconditional: liveliness # is not a preference, it ships in whatever form the mode # supports (no speculative opt-out knob). anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) if anchor: md["thread_id"] = anchor - # Rich status parity (QA-1): run.py's live-status lane stashes the + # Rich status parity: run.py's live-status lane stashes the # current per-tool phrase via set_status_text() (base class store). # Carry it as the typing frame's content so the connector's Slack # sender renders it on assistant.threads.setStatus — the same phrase @@ -1081,7 +1081,7 @@ class RelayAdapter(BasePlatformAdapter): platform = self._platform_by_chat.get(str(chat_id)) if platform != Platform.SLACK.value: return - # Clear must target the SAME thread the heartbeat set (QA-1): apply + # Clear must target the SAME thread the heartbeat set: apply # the identical synthetic-anchor rule as send_typing, or the clear # frame no-ops threadless and the status line sticks until Slack's # own timeout. From 277fc97a0a16e57d04814e41fb9d32ae83332743 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Tue, 28 Jul 2026 11:34:59 +0000 Subject: [PATCH 17/20] =?UTF-8?q?feat(relay):=20dm=5Ftop=5Flevel=5Fthreads?= =?UTF-8?q?=5Fas=5Fsessions=20escape=20hatch=20=E2=80=94=20native=20sessio?= =?UTF-8?q?n-keying=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding: native gates per-message DM sessions behind platforms.slack.extra.dm_top_level_threads_as_sessions; the relay lane coupled session keying to reply_in_thread alone, so 'threaded replies + one rolling session' was expressible on native but not here. Adds the same knob to the relay subset (platforms.relay.extra.slack. dm_top_level_threads_as_sessions, default true = per-message sessions, unchanged behavior). false keeps thread-per-message reply placement but skips the session stamp — one rolling DM session, legacy steer posture. TDD: opt-out + default-unchanged tests written first. --- docs/relay-connector-contract.md | 3 +- gateway/relay/adapter.py | 22 ++++++++++++ .../relay/test_relay_slack_prompt_dm_root.py | 34 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/docs/relay-connector-contract.md b/docs/relay-connector-contract.md index e13d51459dd..91ae62b69e0 100644 --- a/docs/relay-connector-contract.md +++ b/docs/relay-connector-contract.md @@ -736,7 +736,8 @@ Current controls (Slack): | Key | Default | Effect | | --- | --- | --- | -| `reply_in_thread` | `true` | `true`: thread-per-message — each top-level DM message anchors its own thread (status, progress, prompts, final reply all carry that `metadata.thread_id`) and keys its own session, so concurrent messages run in parallel. `false`: flat rolling DM — send-lane frames carry NO thread anchor (stripped, not omitted), one shared session per DM. | +| `reply_in_thread` | `true` | `true`: thread-per-message — each top-level DM message anchors its own thread (status, progress, prompts, final reply all carry that `metadata.thread_id`). `false`: flat rolling DM — send-lane frames carry NO thread anchor (stripped, not omitted), one shared session per DM. | +| `dm_top_level_threads_as_sessions` | `true` | Native-parity escape hatch (mirrors `platforms.slack.extra.dm_top_level_threads_as_sessions`). `true`: in thread-per-message mode each top-level DM message keys its own session, so concurrent messages run in parallel. `false`: threaded reply placement is kept but the session stamp is skipped — one rolling DM session (legacy steer/queue posture). No effect in flat mode, which always keeps the single rolling session. | Typing/status frames always carry the triggering-ts anchor when one is known (liveliness is unconditional, both modes): Slack's status line is diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 8e127ac1f07..71eb326bc72 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -311,6 +311,26 @@ class RelayAdapter(BasePlatformAdapter): except Exception: # noqa: BLE001 - config shape is operator-owned return True + def _dm_top_level_threads_as_sessions(self) -> bool: + """Native-parity escape hatch: per-message DM sessions on/off. + + Mirrors native SlackAdapter._dm_top_level_threads_as_sessions + (platforms.slack.extra.dm_top_level_threads_as_sessions). Default + True: in thread-per-message mode each top-level DM message keys its + own session (parallel turns). Set + platforms.relay.extra.slack.dm_top_level_threads_as_sessions: false + to keep threaded reply PLACEMENT but ONE rolling DM session — the + legacy steer/queue posture, decoupled from reply_in_thread. + """ + try: + return bool( + self._relay_slack_extra().get( + "dm_top_level_threads_as_sessions", True + ) + ) + except Exception: # noqa: BLE001 - config shape is operator-owned + return True + def _stamp_slack_session_thread(self, event) -> None: """Native session-keying parity for fronted Slack DMs. @@ -345,6 +365,8 @@ class RelayAdapter(BasePlatformAdapter): return if not self._effective_reply_in_thread(): return + if not self._dm_top_level_threads_as_sessions(): + return # opt-out: threaded replies, one rolling session src.thread_id = str(message_id) except Exception: # noqa: BLE001 - session stamping must never break inbound logger.debug("slack session-thread stamp failed", exc_info=True) diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index eabc3b2d412..56869f4c6fa 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -450,3 +450,37 @@ def test_run_py_keeps_self_anchor_in_thread_mode(): _resolve_progress_thread_id("slack", None, "1700.001", reply_in_thread=True) == "1700.001" ) + + +# --------------------------------------------------------------------------- +# Native parity escape hatch: platforms.relay.extra.slack. +# dm_top_level_threads_as_sessions=false keeps threaded replies but ONE +# rolling DM session (mirrors native SlackAdapter._dm_top_level_threads_as_sessions). +# Without the knob, reply_in_thread alone couples placement AND session +# keying — a posture native operators can express and relay ones could not. +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_session_stamp_opt_out_keeps_rolling_dm_session(): + adapter, stub = _wire("D1", "dm") + adapter.config.extra = { + "slack": { + "reply_in_thread": True, + "dm_top_level_threads_as_sessions": False, + } + } + event = _inbound_event("D1", message_id="1700.0001", thread_id=None) + adapter._stamp_slack_session_thread(event) + assert getattr(event.source, "thread_id", None) is None, ( + "opt-out: top-level DM must NOT be stamped — one rolling session" + ) + + +@pytest.mark.asyncio +async def test_session_stamp_default_remains_per_message(): + adapter, stub = _wire("D1", "dm") + adapter.config.extra = {"slack": {"reply_in_thread": True}} + event = _inbound_event("D1", message_id="1700.0002", thread_id=None) + adapter._stamp_slack_session_thread(event) + assert getattr(event.source, "thread_id", None) == "1700.0002", ( + "default (native parity): per-message sessions stay on" + ) From 33833e232e5195ce55722bafa50701c52aa24365 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Wed, 29 Jul 2026 12:03:00 -0700 Subject: [PATCH 18/20] fix(relay): apply the Slack thread anchor on the media lane too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DM thread-anchor contract was resolved only in send(). _send_media() — backing send_image, send_image_file, send_voice, send_video and send_document — passed reply_to straight to the frame and never touched metadata, so attachments egressing through the same connector-side Slack sender got both failure shapes this branch set out to remove: flat mode → reply_to survives, the image threads UNDER the user's DM message (the original reported symptom) thread mode → no metadata.thread_id, and threadTs() never reads reply_to, so the image lands in the home channel instead of the per-message thread Both are reachable: gateway/run.py delivers agent artifacts through send_voice/send_document. Extract the three steps that must always happen together (mode gate, mirrored reply_to_message_id strip, metadata promotion) into _apply_slack_thread_anchor and route BOTH lanes through it, so text and media cannot drift again. The media lane copies caller metadata rather than mutating it — these helpers are called in loops with a shared mapping. Also fold send_typing/stop_typing's duplicated status-anchor blocks into _with_status_thread_anchor. They had already drifted (stop_typing omitted the platform check) and the clear must target the thread the heartbeat set or the status line sticks until Slack's own timeout. Tests: media lane pinned in both modes plus the channel and no-caller-mutation cases; verified as real by reverting the fix and watching them fail. --- gateway/relay/adapter.py | 148 +++++++++++------- .../relay/test_relay_slack_dm_streaming.py | 98 +++++++++++- 2 files changed, 191 insertions(+), 55 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index d88815d676a..7c61ff4763d 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -904,32 +904,12 @@ class RelayAdapter(BasePlatformAdapter): if self._transport is None: return SendResult(success=False, error="no transport") # Native _resolve_thread_ts parity: a Slack DM reply must post flat at - # the DM root, not threaded under the triggering message. Drop the - # synthetic self-anchor from BOTH the top-level reply_to and the mirrored - # metadata.reply_to_message_id so the connector can't thread on either. - effective_reply_to = self._resolve_reply_to_for_send( + # the DM root, not threaded under the triggering message. One shared + # helper resolves the anchor for EVERY egress lane (see + # _apply_slack_thread_anchor) so the text and media lanes cannot drift. + effective_reply_to = self._apply_slack_thread_anchor( chat_id, reply_to, send_metadata ) - if effective_reply_to is None and reply_to is not None: - send_metadata.pop("reply_to_message_id", None) - # The connector's Slack sender THREADS ON METADATA ONLY — - # threadTs() reads metadata.thread_id/thread_ts and never looks at - # the frame's reply_to. A send whose only threading signal is - # reply_to (base.py's final-reply and fallback lanes build metadata - # from source.thread_id = None for a top-level DM) would post to the - # home channel even though _resolve_reply_to_for_send kept the - # anchor. Promote the surviving anchor into metadata.thread_id so - # the wire carries it where the connector actually reads it. Only - # when the mode gate kept the anchor (thread-per-message / real - # thread) — flat mode already nulled effective_reply_to above. - if ( - effective_reply_to is not None - and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value - and not ( - send_metadata.get("thread_id") or send_metadata.get("thread_ts") - ) - ): - send_metadata["thread_id"] = str(effective_reply_to) result = await self._transport.send_outbound( { "op": "send", @@ -1010,6 +990,80 @@ class RelayAdapter(BasePlatformAdapter): # Flat mode: synthetic DM self-anchor — post flat at the DM root. return None + def _apply_slack_thread_anchor( + self, + chat_id: str, + reply_to: Optional[str], + metadata: Dict[str, Any], + *, + mirror_key: str = "reply_to_message_id", + ) -> Optional[str]: + """Resolve the outbound Slack thread anchor for ONE egress frame. + + The single choke point every send lane goes through — text (``send``) + and media (``send_media``) alike. It does three things that must always + happen together, and previously only happened on the text lane: + + 1. Mode gate: ``_resolve_reply_to_for_send`` drops the synthetic DM + self-anchor in flat mode, keeps it in thread-per-message mode. + 2. Mirror strip: when the anchor is dropped, remove the mirrored + ``metadata.reply_to_message_id`` too, so the connector cannot + thread on the copy we forgot about. + 3. Anchor promotion: the connector's Slack sender THREADS ON METADATA + ONLY — ``threadTs()`` reads ``metadata.thread_id``/``thread_ts`` + and never looks at the frame's ``reply_to``. A surviving anchor is + promoted into ``metadata.thread_id`` or the message silently lands + in the home channel instead of the per-message thread. + + ``metadata`` is mutated in place; the effective ``reply_to`` is + returned. Non-Slack and non-DM chats are untouched by (1), and (3) is + Slack-only, so other fronted platforms keep their existing behaviour. + """ + effective_reply_to = self._resolve_reply_to_for_send( + chat_id, reply_to, metadata + ) + if effective_reply_to is None and reply_to is not None: + metadata.pop(mirror_key, None) + if ( + effective_reply_to is not None + and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value + and not (metadata.get("thread_id") or metadata.get("thread_ts")) + ): + metadata["thread_id"] = str(effective_reply_to) + return effective_reply_to + + def _with_status_thread_anchor( + self, chat_id: str, metadata: Optional[Dict[str, Any]] + ) -> Dict[str, Any]: + """Copy ``metadata`` with the typing/status thread anchor applied. + + Slack's status line is THREAD-scoped: the connector's typing case + no-ops without a thread anchor, and the typing lane's metadata + (base.py ``_thread_metadata_for_source``) carries none for a top-level + DM (``source.thread_id`` is None). Synthesize it from the per-chat + inbound-ts cache, exactly as native ``send_typing`` resolves + ``thread_ts`` from ``metadata.message_id``. + + Unconditional across both modes: in flat mode the send lane strips its + own anchors (see ``_apply_slack_thread_anchor``), so the status anchor + cannot leak into reply placement, and ``setStatus`` clears without + leaving a message artifact. + + Shared by ``send_typing`` and ``stop_typing`` — the clear MUST target + the same thread the heartbeat set, or the status line sticks until + Slack's own timeout. Keeping one implementation is what guarantees it. + """ + md = dict(metadata or {}) + if ( + not (md.get("thread_id") or md.get("thread_ts")) + and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value + and self._chat_type_by_chat.get(str(chat_id)) == "dm" + ): + anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) + if anchor: + md["thread_id"] = anchor + return md + async def edit_message( self, chat_id: str, @@ -1079,22 +1133,7 @@ class RelayAdapter(BasePlatformAdapter): # like native send_typing resolves thread_ts from metadata.message_id. # Flat mode (reply_in_thread=false) keeps the no-anchor no-op: there # is no thread and must not be one (#18859). - md = dict(metadata or {}) - if ( - not (md.get("thread_id") or md.get("thread_ts")) - and self._platform_by_chat.get(str(chat_id)) == Platform.SLACK.value - and self._chat_type_by_chat.get(str(chat_id)) == "dm" - ): - # Thread mode: status targets the per-message thread. - # Flat mode: the status STILL anchors to the triggering ts — - # setStatus renders in the footer space and clears without a - # message artifact, and flat sends strip their anchors - # so reply placement cannot inherit it. Unconditional: liveliness - # is not a preference, it ships in whatever form the mode - # supports (no speculative opt-out knob). - anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) - if anchor: - md["thread_id"] = anchor + md = self._with_status_thread_anchor(chat_id, metadata) # Rich status parity: run.py's live-status lane stashes the # current per-tool phrase via set_status_text() (base class store). # Carry it as the typing frame's content so the connector's Slack @@ -1143,18 +1182,10 @@ class RelayAdapter(BasePlatformAdapter): platform = self._platform_by_chat.get(str(chat_id)) if platform != Platform.SLACK.value: return - # Clear must target the SAME thread the heartbeat set: apply - # the identical synthetic-anchor rule as send_typing, or the clear - # frame no-ops threadless and the status line sticks until Slack's - # own timeout. - md = dict(metadata or {}) - if ( - not (md.get("thread_id") or md.get("thread_ts")) - and self._chat_type_by_chat.get(str(chat_id)) == "dm" - ): - anchor = self._last_inbound_ts_by_chat.get(str(chat_id)) - if anchor: - md["thread_id"] = anchor + # Clear must target the SAME thread the heartbeat set, or the clear + # frame no-ops threadless and the status line sticks until Slack's own + # timeout. Shared helper with send_typing so the two guards cannot drift. + md = self._with_status_thread_anchor(chat_id, metadata) try: await self._transport.send_outbound( { @@ -1286,14 +1317,23 @@ class RelayAdapter(BasePlatformAdapter): if not uploaded: return None source_url = uploaded + # Same Slack thread-anchor contract as the text lane (send). Media + # frames egress through the connector's Slack sender too, so an + # unresolved anchor threads an image under the user's DM message in + # flat mode, and loses the per-message thread entirely in thread mode + # (threadTs() reads metadata only). Route through the shared helper. + media_metadata: Dict[str, Any] = dict(metadata or {}) + effective_reply_to = self._apply_slack_thread_anchor( + chat_id, reply_to, media_metadata + ) action: Dict[str, Any] = { "op": "send_media", "chat_id": chat_id, "media_kind": media_kind, "source_url": source_url, "content": caption or "", - "reply_to": reply_to, - "metadata": self._with_scope(chat_id, metadata), + "reply_to": effective_reply_to, + "metadata": self._with_scope(chat_id, media_metadata), } if filename: action["filename"] = filename diff --git a/tests/gateway/relay/test_relay_slack_dm_streaming.py b/tests/gateway/relay/test_relay_slack_dm_streaming.py index 2438609d89c..a812f358ec3 100644 --- a/tests/gateway/relay/test_relay_slack_dm_streaming.py +++ b/tests/gateway/relay/test_relay_slack_dm_streaming.py @@ -87,7 +87,7 @@ async def test_slack_dm_reply_keeps_anchor_in_thread_per_message_mode(): assert frame["reply_to"] == "1700.0001", ( "thread-per-message: the triggering ts anchors the final reply" ) - # QA-7: the connector's Slack sender threads on metadata.thread_id ONLY + # The connector's Slack sender threads on metadata.thread_id ONLY # (threadTs() never reads the frame's reply_to), so the surviving anchor # must be promoted into metadata for the send to actually thread. assert (frame["metadata"] or {}).get("thread_id") == "1700.0001" @@ -263,3 +263,99 @@ async def test_slack_dm_stream_consumer_threads_in_thread_per_message_mode(): assert consumer.message_id and consumer.message_id != "__no_edit__" edit_ids = {f["message_id"] for f in stub.sent if f["op"] == "edit"} assert edit_ids <= {stub.next_send_result["message_id"]} + + +# --------------------------------------------------------------------------- +# The media lane obeys the SAME thread-anchor contract as the text lane. +# +# send() and _send_media() both egress through the connector's Slack sender, +# so an anchor resolved on only one of them threads an image under the user's +# DM message in flat mode, and loses the per-message thread in thread mode +# (threadTs() reads metadata only). Both lanes route through +# _apply_slack_thread_anchor; these pin that they stay in agreement. +# --------------------------------------------------------------------------- +def _media_wire(chat_id: str, chat_type: str): + """Like _wire, but the descriptor advertises the send_media op.""" + desc = _slack_desc(supported_ops=("send", "edit", "typing", "send_media")) + stub = StubConnector(desc) + adapter = RelayAdapter(PlatformConfig(), desc, transport=stub) + src = SessionSource( + platform=Platform.SLACK, + chat_id=chat_id, + chat_type=chat_type, + user_id="U1", + ) + adapter._capture_scope( + MessageEvent(text="hi", source=src, message_type=MessageType.TEXT) + ) + return adapter, stub + + +@pytest.mark.asyncio +async def test_slack_dm_media_keeps_and_promotes_anchor_in_thread_mode(): + """Thread-per-message: an image must land in the per-message thread. The + connector threads on metadata.thread_id only, so the surviving anchor has + to be promoted there — a bare reply_to would post to the home channel.""" + adapter, stub = _media_wire("D1", "dm") + await adapter.send_image("D1", "https://example.com/x.png", reply_to="1700.0001") + frame = [f for f in stub.sent if f["op"] == "send_media"][-1] + assert frame["reply_to"] == "1700.0001" + assert (frame["metadata"] or {}).get("thread_id") == "1700.0001", ( + "media frame must carry the anchor where the connector reads it" + ) + + +@pytest.mark.asyncio +async def test_slack_dm_media_drops_synthetic_anchor_in_flat_mode(): + """Flat mode: the media frame drops the synthetic self-anchor exactly as + the text lane does, so the image posts flat at the DM root instead of + threading under the user's message, and invents no thread (#18859).""" + adapter, stub = _media_wire("D1", "dm") + adapter.config.extra = {"reply_in_thread": False} + await adapter.send_image("D1", "https://example.com/x.png", reply_to="1700.0001") + frame = [f for f in stub.sent if f["op"] == "send_media"][-1] + assert frame["reply_to"] is None + assert "thread_id" not in (frame["metadata"] or {}) + assert "thread_ts" not in (frame["metadata"] or {}) + + +@pytest.mark.asyncio +async def test_slack_channel_media_anchor_untouched(): + """Non-DM chats are outside the synthetic-anchor rule: a channel media + send keeps its reply_to unchanged.""" + adapter, stub = _media_wire("C1", "channel") + await adapter.send_image("C1", "https://example.com/x.png", reply_to="1700.0009") + frame = [f for f in stub.sent if f["op"] == "send_media"][-1] + assert frame["reply_to"] == "1700.0009" + + +@pytest.mark.asyncio +async def test_media_caller_metadata_not_mutated(): + """The anchor promotion must not leak into the caller's dict — media + helpers are called in loops with a shared metadata mapping.""" + adapter, stub = _media_wire("D1", "dm") + caller_md = {"user_id": "U1"} + await adapter.send_image( + "D1", "https://example.com/x.png", reply_to="1700.0001", metadata=caller_md + ) + assert caller_md == {"user_id": "U1"}, "caller metadata was mutated in place" + + +# --------------------------------------------------------------------------- +# The status clear targets the same thread the heartbeat set. +# --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_typing_and_clear_share_one_status_anchor(): + """send_typing and stop_typing resolve the anchor through one helper: a + clear that no-ops threadless leaves the status line stuck until Slack's + own timeout.""" + adapter, stub = _wire("D1", "dm") + adapter._last_inbound_ts_by_chat["D1"] = "1700.0001" + await adapter.send_typing("D1") + await adapter.stop_typing("D1") + typing_frames = [f for f in stub.sent if f["op"] == "typing"] + assert len(typing_frames) == 2 + anchors = [(f["metadata"] or {}).get("thread_id") for f in typing_frames] + assert anchors == ["1700.0001", "1700.0001"], ( + "the clear must target the thread the heartbeat set" + ) From 0dc293f4f774a2a17d50b7bb0c389f2559d317d4 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Wed, 29 Jul 2026 12:03:01 -0700 Subject: [PATCH 19/20] fix(relay): coerce Slack behavior flags exactly as the native adapter does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both relay Slack knobs read their value through bool(), while the native adapter they mirror uses str(raw).strip().lower() in {"1","true","yes","on"}. A YAML-quoted string diverges: dm_top_level_threads_as_sessions: "false" → relay True, native False Non-empty strings are truthy, so the escape hatch is silently ignored in exactly the shape an operator writes to switch it OFF. reply_in_thread has the same defect and gates reply placement, session keying and run.py's progress resolver, so one quoted "false" misfires three ways. Route both through a shared _coerce_flag mirroring native's predicate. Real booleans pass through untouched; None falls back to the default. Contract §8 documents the accepted spellings. Tests: both knobs parametrized over the true/false spellings native accepts, plus the absent-key default. --- docs/relay-connector-contract.md | 11 +++++ gateway/relay/adapter.py | 29 ++++++++++--- .../relay/test_relay_slack_dm_streaming.py | 42 +++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/docs/relay-connector-contract.md b/docs/relay-connector-contract.md index 91ae62b69e0..184ce905e65 100644 --- a/docs/relay-connector-contract.md +++ b/docs/relay-connector-contract.md @@ -731,6 +731,10 @@ platforms: Resolution: nested `extra.` object wins → legacy flat key on `extra` honored as fallback → default. Source of truth: `RelayAdapter._effective_reply_in_thread` (`gateway/relay/adapter.py`). +Values coerce exactly as the native Slack adapter's do — `1/true/yes/on` +(case-insensitive, whitespace-trimmed) are ON, anything else is OFF — so a +YAML-quoted `"false"` turns a knob off rather than being read as a truthy +string. Current controls (Slack): @@ -745,6 +749,13 @@ thread-scoped, and in flat mode the send-side anchor strip guarantees the status anchor can never leak into reply placement. Semantics of the native key: see `website/docs/user-guide/messaging/slack.md`. +Thread-anchor resolution applies to EVERY send lane — text (`send`) and media +(`send_media`) alike — through one choke point +(`RelayAdapter._apply_slack_thread_anchor`). Media frames egress via the same +connector-side Slack sender, which threads on `metadata.thread_id` only, so an +attachment resolves its anchor identically to a text reply: promoted into +metadata in thread-per-message mode, stripped in flat mode. + Changes take effect on gateway restart; no connector involvement. --- diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 7c61ff4763d..f4170d16444 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -344,10 +344,30 @@ class RelayAdapter(BasePlatformAdapter): sub = extra.get("slack") return sub if isinstance(sub, dict) else extra + @staticmethod + def _coerce_flag(raw: Any, default: bool) -> bool: + """Coerce an operator-supplied boolean exactly as native Slack does. + + Native SlackAdapter reads its behavior flags with + ``str(raw).strip().lower() in {"1","true","yes","on"}``, so a + YAML-quoted ``"false"`` — a shape operators write routinely — turns + the flag OFF. A bare ``bool()`` would read that same string as True + (non-empty string), silently ignoring the off switch. These knobs are + documented as native-parity mirrors, so they must coerce identically + or the parity claim only holds for unquoted YAML booleans. + """ + if raw is None: + return default + if isinstance(raw, bool): + return raw + return str(raw).strip().lower() in {"1", "true", "yes", "on"} + def _effective_reply_in_thread(self) -> bool: """Resolve the thread-per-message vs flat-DM mode for fronted Slack.""" try: - return bool(self._relay_slack_extra().get("reply_in_thread", True)) + return self._coerce_flag( + self._relay_slack_extra().get("reply_in_thread"), True + ) except Exception: # noqa: BLE001 - config shape is operator-owned return True @@ -363,10 +383,9 @@ class RelayAdapter(BasePlatformAdapter): legacy steer/queue posture, decoupled from reply_in_thread. """ try: - return bool( - self._relay_slack_extra().get( - "dm_top_level_threads_as_sessions", True - ) + return self._coerce_flag( + self._relay_slack_extra().get("dm_top_level_threads_as_sessions"), + True, ) except Exception: # noqa: BLE001 - config shape is operator-owned return True diff --git a/tests/gateway/relay/test_relay_slack_dm_streaming.py b/tests/gateway/relay/test_relay_slack_dm_streaming.py index a812f358ec3..9969b739575 100644 --- a/tests/gateway/relay/test_relay_slack_dm_streaming.py +++ b/tests/gateway/relay/test_relay_slack_dm_streaming.py @@ -341,6 +341,48 @@ async def test_media_caller_metadata_not_mutated(): assert caller_md == {"user_id": "U1"}, "caller metadata was mutated in place" +# --------------------------------------------------------------------------- +# Operator flags coerce exactly as the native Slack adapter's do. +# --------------------------------------------------------------------------- +@pytest.mark.parametrize( + "raw,expected", + [ + (False, False), + ("false", False), + ("False", False), + (" no ", False), + ("off", False), + ("0", False), + (True, True), + ("true", True), + ("yes", True), + ("on", True), + ("1", True), + ], +) +def test_relay_slack_flags_coerce_like_native(raw, expected): + """A YAML-quoted "false" must turn these knobs OFF, matching native's + str().strip().lower() predicate. A bare bool() would read any non-empty + string as True and silently ignore the operator's off switch.""" + adapter, _stub = _wire("D1", "dm") + adapter.config.extra = { + "slack": { + "reply_in_thread": raw, + "dm_top_level_threads_as_sessions": raw, + } + } + assert adapter._effective_reply_in_thread() is expected + assert adapter._dm_top_level_threads_as_sessions() is expected + + +def test_relay_slack_flags_default_true_when_absent(): + """Both knobs default ON when the operator sets nothing.""" + adapter, _stub = _wire("D1", "dm") + adapter.config.extra = {} + assert adapter._effective_reply_in_thread() is True + assert adapter._dm_top_level_threads_as_sessions() is True + + # --------------------------------------------------------------------------- # The status clear targets the same thread the heartbeat set. # --------------------------------------------------------------------------- From b521fd9dc929e005343391b66025d69f085b8519 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Wed, 29 Jul 2026 12:03:01 -0700 Subject: [PATCH 20/20] docs(relay): finish the QA-N scrub in the new relay test files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier scrub covered adapter.py; nine internal QA-campaign tracker IDs remained in the two new test files, including a module docstring and an assertion message. They mean nothing to a future reader — describe the behavior instead. Comments only, no assertion changes. --- .../relay/test_relay_slack_prompt_dm_root.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py index 56869f4c6fa..466fd10071c 100644 --- a/tests/gateway/relay/test_relay_slack_prompt_dm_root.py +++ b/tests/gateway/relay/test_relay_slack_prompt_dm_root.py @@ -1,4 +1,4 @@ -"""Slack relay: interactive prompts follow the turn's thread stamp (QA-5). +"""Slack relay: interactive prompts follow the turn's thread stamp. The threading MODE (flat DM vs thread-per-message) is decided in exactly ONE place: run.py's ``_resolve_progress_thread_id``, which reads @@ -213,7 +213,7 @@ async def test_non_slack_dm_approval_keeps_thread_id(): # --------------------------------------------------------------------------- -# QA-1 rich status: the relay advertises Slack's text status line and carries +# Rich status: the relay advertises Slack's text status line and carries # the live per-tool phrase on the typing frame (native set_status_text parity). # --------------------------------------------------------------------------- @pytest.mark.asyncio @@ -251,7 +251,7 @@ async def test_typing_carries_live_status_phrase(): # --------------------------------------------------------------------------- -# QA-1 status thread anchor: typing frames synthesize the per-message thread +# Status thread anchor: typing frames synthesize the per-message thread # root in thread-per-message mode (the status line is thread-only on Slack). # --------------------------------------------------------------------------- def _wire_with_ts(chat_id, chat_type, message_id, **kw): @@ -282,7 +282,7 @@ async def test_typing_synthesizes_thread_anchor_in_thread_mode(): async def test_typing_flat_mode_status_anchors_to_trigger_ts_by_default(): """Flat-DM liveliness: the STATUS still anchors to the triggering ts (renders in the footer space, no message artifact) while replies stay - flat — QA-6/7 strip send anchors, so placement cannot inherit this.""" + flat — the send lane strips its anchors, so placement cannot inherit this.""" adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") adapter.config.extra = {"reply_in_thread": False} await adapter.send_typing("D1", metadata=None) @@ -294,7 +294,7 @@ async def test_typing_flat_mode_status_anchors_to_trigger_ts_by_default(): async def test_typing_anchors_unconditionally_in_both_modes(): """Liveliness is not a preference: the status anchors whenever an inbound ts exists, regardless of reply_in_thread. Placement safety comes from the - QA-6/7 send-side anchor strip, not from suppressing the status.""" + send-side anchor strip, not from suppressing the status.""" for extra in ({}, {"slack": {"reply_in_thread": False}}): adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") adapter.config.extra = extra @@ -306,7 +306,7 @@ async def test_typing_anchors_unconditionally_in_both_modes(): @pytest.mark.asyncio async def test_flat_mode_sends_stay_flat_with_status_anchor_active(): """The liveliness anchor must NOT leak into reply placement: sends in - flat mode still strip the synthetic anchor (QA-6/7 contract).""" + flat mode still strip the synthetic anchor (send-lane contract).""" adapter, stub = _wire_with_ts("D1", "dm", "1700.0042") adapter.config.extra = {"reply_in_thread": False} await adapter.send_typing("D1", metadata=None) @@ -339,7 +339,7 @@ async def test_stop_typing_clear_targets_same_synthesized_thread(): # --------------------------------------------------------------------------- -# QA-3 session keying: a top-level Slack DM message gets its own ts stamped as +# Session keying: a top-level Slack DM message gets its own ts stamped as # source.thread_id (native inbound parity) so each message keys a FRESH # session in thread-per-message mode; flat mode and real threads untouched. # --------------------------------------------------------------------------- @@ -370,7 +370,7 @@ def test_two_top_level_messages_key_distinct_sessions(): adapter._stamp_slack_session_thread(e2) k1 = build_session_key(e1.source) k2 = build_session_key(e2.source) - assert k1 != k2, "each top-level message must be its own session (QA-3)" + assert k1 != k2, "each top-level message must be its own session" def test_real_thread_reply_keeps_its_thread_session():