feat(mattermost): isolate DM sessions per top-level message (Slack parity) - #80530
Open
weslien wants to merge 1 commit into
Open
feat(mattermost): isolate DM sessions per top-level message (Slack parity)#80530weslien wants to merge 1 commit into
weslien wants to merge 1 commit into
Conversation
Mattermost top-level DM messages shared one continuous session because the adapter only stamped thread_id for non-DM channels (fix for NousResearch#18279). In DMs the adapter left thread_id unset, so build_session_key() keyed every top-level DM message by channel only — collapsing unrelated conversations into one session and blocking parallel DM conversations. Slack solved this with dm_top_level_threads_as_sessions (defaults true): each top-level DM message is stamped with its own message ts as thread_id, giving each root message its own session. Threaded replies keep the thread's root_id and continue that session. This ports the same behavior to Mattermost: - New _dm_top_level_threads_as_sessions() method mirrors Slack's exactly (platforms.mattermost.extra.dm_top_level_threads_as_sessions, default true) - Top-level DM messages (channel_type D, no root_id) get stamped with their own post_id as thread_id → unique session per message - Threaded DM replies keep root_id → continue the root's session - Configurable: set to false to revert to legacy single-session-per-DM The channel stamping logic is unchanged — only the DM exclusion is lifted. Independent of reply_mode (works in both thread and off modes), because DM session isolation is orthogonal to reply placement.
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Ports Slack's
dm_top_level_threads_as_sessionssession isolation to the Mattermost adapter. Each top-level DM message now gets its own session (stamped with its ownpost_idasthread_id), while threaded replies continue the root's session — matching the behavior Slack has had sincedm_top_level_threads_as_sessionswas introduced there.Problem: Mattermost top-level DM messages shared one continuous session. The channel stamping fix (#18279) explicitly excluded DMs (
channel_type_raw != "D"), so every top-level DM in the same channel collapsed intosession_key = platform:dm:{chat_id}— no thread suffix. This blocks parallel DM conversations and mixes unrelated topics into one context window.Fix: Stamp top-level DM messages with their own
post_idasthread_id, gated by a new_dm_top_level_threads_as_sessions()config method that mirrors Slack's exactly.Related Issue
Closes #18279 (the DM portion — the channel portion was already fixed; this completes the parity).
Type of Change
Changes Made
plugins/platforms/mattermost/adapter.py:_dm_top_level_threads_as_sessions()— readsplatforms.mattermost.extra.dm_top_level_threads_as_sessions, defaults totrue(mirrors Slack's default)channel_type == "D", noroot_id) getthread_id = post_idwhen the feature is enabledtests/gateway/test_mattermost.py:test_dm_top_level_post_gets_own_thread_id_by_default— top-level DM gets its own thread_idtest_dm_reply_in_thread_keeps_root_id— threaded reply keeps root_id (continues session)test_dm_top_level_disabled_when_config_false— config false → no stamping (legacy behavior)test_dm_root_and_reply_share_session_key— end-to-end: root + reply produce samebuild_session_key()How to Test
All 27 tests pass (23 existing + 4 new).
Checklist
Code
feat(mattermost): ...)pytest tests/gateway/test_mattermost.py -vand all tests passDocumentation & Housekeeping
dm_top_level_threads_as_sessionsis anextrakey not documented incli-config.yaml.example, consistent with Slack's treatment)cli-config.yaml.example— N/A (extra key, matches Slack convention)CONTRIBUTING.mdorAGENTS.md— N/ADesign Notes
Why gate with a config option instead of always-on? Slack uses the same pattern — default
truewith an escape hatch. Some deployments may rely on the legacy single-session-per-DM behavior (e.g., a DM channel used as a persistent command surface). The config gate lets them revert without a code change.Why independent of
reply_mode? DM session isolation is orthogonal to reply placement. Whether the bot replies in threads or flat, each top-level DM message should still get its own session. Thereply_modeflag controls outbound formatting; session keying is inbound routing.