fix(discord): anchor inbound message-map on thread root so directly-created threads keep threading - #319
Conversation
…reated threads keep threading Discord's direct "Create Thread" flow mints a thread channel id unrelated to any message id. _handle_inbound_message recorded the message-map correlation under the message's OWN ref, while the thread lookup keys on msg.root_id (the thread id). The two only coincide when a thread is created FROM an existing message (reply-in-thread), so a directly-created thread's second message can never resolve its predecessor and every reply after the first posts top-level in Matrix. Anchor the recorded correlation on msg.root_id or msg.message_ref instead, and only record when thread_root_id is None (first bridged message of a thread anchors it; re-recording after that would just shadow the resolved root). Mirrors the pattern _handle_inbound_command already uses via cmd.root_id or cmd.message_ref. Added test_bridge_inbound_thread_anchor.py covering the directly-created-thread case (second reply must still thread) and the top-level case (anchors on its own ref, unchanged). Fixes sandbox-quantum#318 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All contributors have signed the CLA. ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LNFAdqF3aeuhZb8beCmC2v
|
Ran the full local pre-PR verification (macOS, Python 3.13.11, uv 0.9.18) at HEAD (cc98ef6):
All green — closing the local-verification gap noted in the PR description. — eq3 (AI agent) for @meganechan |
|
Thanks for this — the write-up in #318 is excellent, and the DB evidence made it quick to confirm. The diagnosis is right: the record keys on the message's own ref, the lookup keys on the thread root, and on Discord those only coincide when the thread was created from a message. I'd like to fix it a bit differently, though — here's why. The awkward constraint: UniqueConstraint("bridge_id", "matrix_event_id"),
UniqueConstraint("bridge_id", "external_post_id"),One Matrix event gets exactly one external post id. So each bridged message has a single key, and that key is doing two different jobs:
On Mattermost and Slack those are the same thing, because a thread root really is a message in the channel. On Discord a thread is a channel, not a message, so they come apart. That's the real defect — one column doing two jobs. Because there's only one key per event, any in-place fix has to pick one job over the other. This PR picks threading: with
So rather than trade one for the other, I'd rather split them:
That fixes Discord with no trade-off and no behaviour change elsewhere. It needs a migration, so it's a bit more than this diff. Would you be up for taking that on? Your test file carries over almost as-is — it's exactly the right case to assert. If you'd rather not, no problem at all, we'll pick it up from here and credit the issue. |
Fixes #318
What
_handle_inbound_messageincore/switch_core/bridges/collaboration/bridge_core.pynow anchors the recorded message-map correlation onmsg.root_id or msg.message_refinstead of alwaysmsg.message_ref, and only records it whenthread_root_id is None(i.e. the first bridged message of a thread).Why
Discord's direct "Create Thread" flow mints a thread whose channel id is unrelated to any message id. The thread lookup (a few lines above the change) already keys on
msg.root_id, but the record was keying on the message's own ref — so a directly-created thread's message-map entry could never be found by the next reply's lookup. Every reply after the first in such a thread posted top-level in Matrix, and agents received those events with nothread_id.The sibling method
_handle_inbound_commandalready anchors correctly (cmd.root_id or cmd.message_ref, line ~622) — this brings_handle_inbound_messagein line with that existing precedent. Non-thread (top-level) messages are unaffected:msg.root_idisNonefor them, so the anchor is unchanged (msg.message_ref).How verified
bridge_message_mapheld rows keyed{channel_id}:{message_id}for messages posted into a directly-created thread, while the lookup for the next reply computed{parent_channel_id}:{thread_channel_id}— a key that was never recorded. Full evidence + repro steps in Discord: directly-created threads lose threading after the first message (message-map keyed on message ref, not thread root) #318.core/tests/switch_core/bridges/collaboration/test_bridge_inbound_thread_anchor.py:test_directly_created_thread_second_reply_still_threads— simulates a thread whose id shares no id with any message (the Discord direct-create case); asserts the anchor recorded after the first reply is the thread id, and the second reply resolves and threads under it.test_top_level_message_still_anchors_on_its_own_ref— regression guard that top-level messages are unaffected.git stashthe fix, rerun) and passes after — confirms the test actually exercises the bug, not just the happy path.uv run ruff checkclean on both changed files.just testsuite (requires a real PostgreSQL instance percore/tests/conventions) orjust typecheck— happy to run these if a maintainer points me at CI, or CI will confirm on this PR.Precedent referenced
_handle_inbound_command, same file, ~line 622:🤖 Filed by eq3 (AI agent) on behalf of Tony (meganechan) — found while self-hosting Switch v0.21.0