Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions core/switch_core/bridges/collaboration/bridge_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,13 @@ async def _handle_inbound_message(self, msg: InboundMessage) -> None:
matrix_room_id,
)

# Where later replies should find this conversation: the thread's own
# key when this message sits inside a thread (so the NEXT reply in a
# directly-created thread resolves), its own ref when top-level (so a
# thread created FROM this post resolves). Mirrors
# _handle_inbound_command, which already anchors on root_id.
thread_anchor_post = msg.root_id or msg.message_ref

# An attachment the platform offered but we could not relay must be
# visible in the room, not swallowed. Append it to the message body so
# both the agent and the humans see that a file went missing.
Expand All @@ -529,12 +536,16 @@ async def _handle_inbound_message(self, msg: InboundMessage) -> None:
matrix_room_id,
)
return
# Record the correlation so a later reply (either direction) threads.
await self._record_message_map(
external_channel_id=msg.channel_id,
matrix_event_id=event_id,
external_post_id=msg.message_ref,
)
# Record the correlation so a later reply (either direction)
# threads. Only the first bridged message of a thread anchors it —
# once the anchor resolves, thread_root_id is set and re-recording
# would just shadow it.
if thread_root_id is None:
await self._record_message_map(
external_channel_id=msg.channel_id,
matrix_event_id=event_id,
external_post_id=thread_anchor_post,
)
return

# Caption convention: the text rides as the caption on the first
Expand Down Expand Up @@ -580,11 +591,11 @@ async def _handle_inbound_message(self, msg: InboundMessage) -> None:
if index == 0:
first_event_id = event_id

if first_event_id is not None:
if first_event_id is not None and thread_root_id is None:
await self._record_message_map(
external_channel_id=msg.channel_id,
matrix_event_id=first_event_id,
external_post_id=msg.message_ref,
external_post_id=thread_anchor_post,
)

async def _handle_inbound_command(self, cmd: InboundCommand) -> None:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
"""A directly-created thread must keep threading past its first reply.

Discord's "Create Thread" flow (not "reply in thread" on an existing message)
mints a thread whose channel id is unrelated to any message id. `msg.root_id`
then carries that thread id, never a message id — so recording the
correlation under the message's own ref (as `_handle_inbound_message` used
to) can never be looked up again by `_matrix_event_for_external_post`, which
keys on `root_id`. `_handle_inbound_command` already anchors on
`root_id or message_ref`; these pin the same anchor for the message path.
"""

from __future__ import annotations

from types import SimpleNamespace
from typing import Any

from switch_core.bridges.collaboration.bridge_core import BridgeCore
from switch_core.bridges.collaboration.models import InboundMessage


class _FakePuppet:
matrix_user_id = "@puppet:s"

def __init__(self) -> None:
self.messages: list[dict[str, Any]] = []
self._next_event = 0

async def send_message(
self, matrix_room_id, content, format=None, thread_root_id=None
): # noqa: ANN001, ANN201, A002
self.messages.append({"content": content, "thread_root_id": thread_root_id})
event_id = f"$evt-{self._next_event}"
self._next_event += 1
return event_id


class _FakeAdapter:
def translate_inbound(self, content: str) -> str:
return content


def _fake_bridge(*, matrix_events: dict[str, str]) -> SimpleNamespace:
"""`matrix_events` maps external post id -> already-bridged Matrix event id."""
puppet = _FakePuppet()
recorded: list[dict[str, str]] = []

async def _is_registered_agent(_name: str) -> bool:
return False

async def _ensure_user_in_matrix_room(**_kwargs: Any) -> _FakePuppet:
return puppet

async def _repair_placeholder_username(*_args: Any, **_kwargs: Any) -> None:
return None

async def _matrix_event_for_external_post(post_id: str) -> str | None:
return matrix_events.get(post_id)

async def _record_message_map(**kwargs: str) -> None:
recorded.append(kwargs)
# A real store: recording makes the post resolvable on the next lookup.
matrix_events[kwargs["external_post_id"]] = kwargs["matrix_event_id"]

return SimpleNamespace(
_repair_placeholder_username=_repair_placeholder_username,
_adapter=_FakeAdapter(),
_channel_to_room={"chan-1": ("room-1", "!room:s")},
_is_registered_agent=_is_registered_agent,
_ensure_user_in_matrix_room=_ensure_user_in_matrix_room,
_matrix_event_for_external_post=_matrix_event_for_external_post,
_record_message_map=_record_message_map,
puppet=puppet,
recorded=recorded,
)


def _msg(
*, message_ref: str, root_id: str | None, content: str = "hi"
) -> InboundMessage:
return InboundMessage(
channel_id="chan-1",
channel_type="channel_public",
sender_id="U1",
sender_name="alice",
content=content,
message_ref=message_ref,
root_id=root_id,
)


async def test_directly_created_thread_second_reply_still_threads() -> None:
# thread id shares no id with any message — the direct-create case.
thread_id = "thread-999"
bridge = _fake_bridge(matrix_events={})

# First reply into the thread: no Matrix event mapped for it yet, so it
# posts top-level, but the anchor recorded must be the THREAD id (root_id)
# so the next reply can find it — not this message's own ref.
await BridgeCore._handle_inbound_message(
bridge, _msg(message_ref="post-1", root_id=thread_id, content="first")
)
assert bridge.puppet.messages[0]["thread_root_id"] is None
assert bridge.recorded == [
{
"external_channel_id": "chan-1",
"matrix_event_id": "$evt-0",
"external_post_id": thread_id,
}
]

# Second reply into the SAME thread now resolves, because the anchor was
# the thread id, not the first message's own ref.
await BridgeCore._handle_inbound_message(
bridge, _msg(message_ref="post-2", root_id=thread_id, content="second")
)
assert bridge.puppet.messages[1]["thread_root_id"] == "$evt-0"
# Anchor already resolved: no re-recording (would just shadow the first).
assert len(bridge.recorded) == 1


async def test_top_level_message_still_anchors_on_its_own_ref() -> None:
bridge = _fake_bridge(matrix_events={})

await BridgeCore._handle_inbound_message(
bridge, _msg(message_ref="post-1", root_id=None, content="top level")
)

assert bridge.recorded == [
{
"external_channel_id": "chan-1",
"matrix_event_id": "$evt-0",
"external_post_id": "post-1",
}
]
Loading